diff --git a/benchmarks/benches/room_bench.rs b/benchmarks/benches/room_bench.rs index e9decee401f..fae41ef7536 100644 --- a/benchmarks/benches/room_bench.rs +++ b/benchmarks/benches/room_bench.rs @@ -63,6 +63,7 @@ pub fn receive_all_members_benchmark(c: &mut Criterion) { .expect("initial filling of sqlite failed"); let base_client = BaseClient::with_store_config(StoreConfig::new().state_store(sqlite_store)); + runtime .block_on(base_client.set_session_meta( SessionMeta { @@ -72,6 +73,7 @@ pub fn receive_all_members_benchmark(c: &mut Criterion) { None, )) .expect("Could not set session meta"); + base_client.get_or_create_room(&room_id, RoomState::Joined); let request = get_member_events::v3::Request::new(room_id.clone()); diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index bf90d8f0fa2..a31d35062b7 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -81,8 +81,8 @@ impl Room { /// Returns the room's name from the state event if available, otherwise /// compute a room name based on the room's nature (DM or not) and number of /// members. - pub fn display_name(&self) -> Result { - Ok(RUNTIME.block_on(self.inner.computed_display_name())?.to_string()) + pub fn display_name(&self) -> Option { + Some(self.inner.cached_display_name()?.to_string()) } /// The raw name as present in the room state event. diff --git a/bindings/matrix-sdk-ffi/src/room_info.rs b/bindings/matrix-sdk-ffi/src/room_info.rs index 9abaeecac74..6e379728956 100644 --- a/bindings/matrix-sdk-ffi/src/room_info.rs +++ b/bindings/matrix-sdk-ffi/src/room_info.rs @@ -69,7 +69,7 @@ impl RoomInfo { Ok(Self { id: room.room_id().to_string(), - display_name: room.computed_display_name().await.ok().map(|name| name.to_string()), + display_name: room.cached_display_name().map(|name| name.to_string()), raw_name: room.name(), topic: room.topic(), avatar_url: room.avatar_url().map(Into::into), diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index 4e76cb075dc..8a3571a064a 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -489,7 +489,7 @@ impl RoomListItem { /// compute a room name based on the room's nature (DM or not) and number of /// members. fn display_name(&self) -> Option { - RUNTIME.block_on(self.inner.computed_display_name()) + self.inner.cached_display_name() } fn avatar_url(&self) -> Option { diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 478153325fb..26adfb424b5 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -751,6 +751,7 @@ impl BaseClient { RoomState::Joined, self.roominfo_update_sender.clone(), ); + if room.state() != RoomState::Joined { let _sync_lock = self.sync_lock().lock().await; @@ -777,6 +778,7 @@ impl BaseClient { RoomState::Left, self.roominfo_update_sender.clone(), ); + if room.state() != RoomState::Left { let _sync_lock = self.sync_lock().lock().await; @@ -852,6 +854,7 @@ impl BaseClient { RoomState::Joined, self.roominfo_update_sender.clone(), ); + let mut room_info = room.clone_info(); room_info.mark_as_joined(); @@ -964,6 +967,7 @@ impl BaseClient { RoomState::Left, self.roominfo_update_sender.clone(), ); + let mut room_info = room.clone_info(); room_info.mark_as_left(); room_info.mark_state_partially_synced(); @@ -1022,6 +1026,7 @@ impl BaseClient { RoomState::Invited, self.roominfo_update_sender.clone(), ); + let mut room_info = room.clone_info(); room_info.mark_as_invited(); room_info.mark_state_fully_synced(); @@ -1066,6 +1071,13 @@ impl BaseClient { self.apply_changes(&changes, false); } + // Now that all the rooms information have been saved, update the display name + // cache (which relies on information stored in the database). This will + // live in memory, until the next sync which will saves the room info to + // disk; we do this to avoid saving that would be redundant with the + // above. Oh well. + new_rooms.update_in_memory_caches(&self.store).await; + info!("Processed a sync response in {:?}", now.elapsed()); let response = SyncResponse { @@ -1655,7 +1667,7 @@ mod tests { let room = client.get_room(room_id).expect("Room not found"); assert_eq!(room.state(), RoomState::Invited); assert_eq!( - room.computed_display_name().await.expect("fetching display name failed"), + room.compute_display_name().await.expect("fetching display name failed"), DisplayName::Calculated("Kyra".to_owned()) ); } diff --git a/crates/matrix-sdk-base/src/rooms/mod.rs b/crates/matrix-sdk-base/src/rooms/mod.rs index 48ef48c12b5..a5fca578bfe 100644 --- a/crates/matrix-sdk-base/src/rooms/mod.rs +++ b/crates/matrix-sdk-base/src/rooms/mod.rs @@ -122,15 +122,6 @@ impl BaseRoomInfo { Self::default() } - pub(crate) fn calculate_room_name( - &self, - joined_member_count: u64, - invited_member_count: u64, - heroes: Vec<&str>, - ) -> DisplayName { - calculate_room_name(joined_member_count, invited_member_count, heroes) - } - /// Get the room version of this room. /// /// For room versions earlier than room version 11, if the event is @@ -359,45 +350,6 @@ impl Default for BaseRoomInfo { } } -/// Calculate room name according to step 3 of the [naming algorithm.] -/// -/// [naming algorithm]: https://spec.matrix.org/latest/client-server-api/#calculating-the-display-name-for-a-room -fn calculate_room_name( - joined_member_count: u64, - invited_member_count: u64, - mut heroes: Vec<&str>, -) -> DisplayName { - let num_heroes = heroes.len() as u64; - let invited_joined = invited_member_count + joined_member_count; - let invited_joined_minus_one = invited_joined.saturating_sub(1); - - // Stabilize ordering. - heroes.sort_unstable(); - - let names = if num_heroes == 0 && invited_joined > 1 { - format!("{} people", invited_joined) - } else if num_heroes >= invited_joined_minus_one { - heroes.join(", ") - } else if num_heroes < invited_joined_minus_one && invited_joined > 1 { - // TODO: What length does the spec want us to use here and in - // the `else`? - format!("{}, and {} others", heroes.join(", "), (invited_joined - num_heroes)) - } else { - "".to_owned() - }; - - // User is alone. - if invited_joined <= 1 { - if names.is_empty() { - DisplayName::Empty - } else { - DisplayName::EmptyWas(names) - } - } else { - DisplayName::Calculated(names) - } -} - /// The content of an `m.room.create` event, with a required `creator` field. /// /// Starting with room version 11, the `creator` field should be removed and the @@ -563,43 +515,7 @@ mod tests { use ruma::events::tag::{TagInfo, TagName, Tags}; - use super::{calculate_room_name, BaseRoomInfo, DisplayName, RoomNotableTags}; - - #[test] - fn test_calculate_room_name() { - let mut actual = calculate_room_name(2, 0, vec!["a"]); - assert_eq!(DisplayName::Calculated("a".to_owned()), actual); - - actual = calculate_room_name(3, 0, vec!["a", "b"]); - assert_eq!(DisplayName::Calculated("a, b".to_owned()), actual); - - actual = calculate_room_name(4, 0, vec!["a", "b", "c"]); - assert_eq!(DisplayName::Calculated("a, b, c".to_owned()), actual); - - actual = calculate_room_name(5, 0, vec!["a", "b", "c"]); - assert_eq!(DisplayName::Calculated("a, b, c, and 2 others".to_owned()), actual); - - actual = calculate_room_name(5, 0, vec![]); - assert_eq!(DisplayName::Calculated("5 people".to_owned()), actual); - - actual = calculate_room_name(0, 0, vec![]); - assert_eq!(DisplayName::Empty, actual); - - actual = calculate_room_name(1, 0, vec![]); - assert_eq!(DisplayName::Empty, actual); - - actual = calculate_room_name(0, 1, vec![]); - assert_eq!(DisplayName::Empty, actual); - - actual = calculate_room_name(1, 0, vec!["a"]); - assert_eq!(DisplayName::EmptyWas("a".to_owned()), actual); - - actual = calculate_room_name(1, 0, vec!["a", "b"]); - assert_eq!(DisplayName::EmptyWas("a, b".to_owned()), actual); - - actual = calculate_room_name(1, 0, vec!["a", "b", "c"]); - assert_eq!(DisplayName::EmptyWas("a, b, c".to_owned()), actual); - } + use super::{BaseRoomInfo, RoomNotableTags}; #[test] fn test_handle_notable_tags_favourite() { diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 28c6ccff035..0c0b5d996ca 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -478,9 +478,125 @@ impl Room { /// /// The display name is calculated according to [this algorithm][spec]. /// + /// This is automatically recomputed on every successful sync, and the + /// cached result can be retrieved in + /// [`Self::cached_display_name`]. + /// /// [spec]: - pub async fn computed_display_name(&self) -> StoreResult { - self.calculate_name().await + pub async fn compute_display_name(&self) -> StoreResult { + let update_cache = |new_val: DisplayName| { + self.inner.update_if(|info| { + if info.cached_display_name.as_ref() != Some(&new_val) { + info.cached_display_name = Some(new_val.clone()); + true + } else { + false + } + }); + new_val + }; + + let summary = { + let inner = self.inner.read(); + + if let Some(name) = inner.name() { + let name = name.trim().to_owned(); + drop(inner); // drop the lock on `self.inner` to avoid deadlocking in `update_cache`. + return Ok(update_cache(DisplayName::Named(name))); + } + + if let Some(alias) = inner.canonical_alias() { + let alias = alias.alias().trim().to_owned(); + drop(inner); // See above comment. + return Ok(update_cache(DisplayName::Aliased(alias))); + } + + inner.summary.clone() + }; + + // From here, use some heroes to compute the room's name. + let own_user_id = self.own_user_id().as_str(); + + let (heroes, num_joined_guess): (Vec, _) = if !summary.heroes_names.is_empty() { + // Straightforward path: pass through the heroes names, don't give a guess of + // the number of members. + (summary.heroes_names, None) + } else if !summary.heroes_user_ids.is_empty() { + // Use the heroes, if available. + let heroes = summary.heroes_user_ids; + + let mut names = Vec::with_capacity(heroes.len()); + for user_id in heroes { + if user_id == own_user_id { + continue; + } + if let Some(member) = self.get_member(&user_id).await? { + names.push(member.name().to_owned()); + } + } + + (names, None) + } else { + let mut joined_members = self.members(RoomMemberships::JOIN).await?; + + // Make the ordering deterministic. + joined_members.sort_unstable_by(|lhs, rhs| lhs.name().cmp(rhs.name())); + + // We can make a good prediction of the total number of members here. This might + // be incorrect if the database info is outdated. + let num_joined = Some(joined_members.len()); + + ( + joined_members + .into_iter() + .take(NUM_HEROES) + .filter_map(|u| (u.user_id() != own_user_id).then(|| u.name().to_owned())) + .collect(), + num_joined, + ) + }; + + let (num_joined, num_invited) = match self.state() { + RoomState::Invited => { + // when we were invited we don't have a proper summary, we have to do best + // guessing + (heroes.len() as u64, 1u64) + } + + RoomState::Joined if summary.joined_member_count == 0 => { + let num_joined = if let Some(num_joined) = num_joined_guess { + num_joined + } else { + self.joined_user_ids().await?.len() + }; + + (num_joined as u64, summary.invited_member_count) + } + + _ => (summary.joined_member_count, summary.invited_member_count), + }; + + debug!( + room_id = ?self.room_id(), + own_user = ?self.own_user_id, + num_joined, num_invited, + heroes = ?heroes, + "Calculating name for a room based on heroes", + ); + + Ok(update_cache(compute_display_name_from_heroes( + num_joined, + num_invited, + heroes.iter().map(|hero| hero.as_str()).collect(), + ))) + } + + /// Returns the cached computed display name, if available. + /// + /// This cache is refilled every time we call + /// [`Self::compute_display_name`]. + pub fn cached_display_name(&self) -> Option { + self.inner.read().cached_display_name.clone() } /// Return the last event in this room, if one has been cached during @@ -608,100 +724,6 @@ impl Room { self.inner.read().joined_members_count() } - async fn calculate_name(&self) -> StoreResult { - let summary = { - let inner = self.inner.read(); - - if let Some(name) = inner.name() { - let name = name.trim(); - return Ok(DisplayName::Named(name.to_owned())); - } - - if let Some(alias) = inner.canonical_alias() { - let alias = alias.alias().trim(); - return Ok(DisplayName::Aliased(alias.to_owned())); - } - - inner.summary.clone() - }; - - let own_user_id = self.own_user_id().as_str(); - - let (heroes, guessed_num_members): (Vec, _) = if !summary.heroes_names.is_empty() { - // Straightforward path: pass through the heroes names, don't give a guess of - // the number of members. - (summary.heroes_names, None) - } else if !summary.heroes_user_ids.is_empty() { - // Use the heroes, if available. - let heroes = summary.heroes_user_ids; - - let mut names = Vec::with_capacity(heroes.len()); - for user_id in heroes { - if user_id == own_user_id { - continue; - } - if let Some(member) = self.get_member(&user_id).await? { - names.push(member.name().to_owned()); - } - } - - (names, None) - } else { - let mut members = self.members(RoomMemberships::ACTIVE).await?; - - // Make the ordering deterministic. - members.sort_unstable_by(|lhs, rhs| lhs.name().cmp(rhs.name())); - - // We can make a good prediction of the total number of members here. This might - // be incorrect if the database info is outdated. - let guessed_num_members = Some(members.len()); - - ( - members - .into_iter() - .take(NUM_HEROES) - .filter_map(|u| (u.user_id() != own_user_id).then(|| u.name().to_owned())) - .collect(), - guessed_num_members, - ) - }; - - let (num_joined, num_invited) = match self.state() { - RoomState::Invited => { - // when we were invited we don't have a proper summary, we have to do best - // guessing - (heroes.len() as u64, 1u64) - } - - RoomState::Joined if summary.joined_member_count == 0 => { - let num_members = if let Some(guessed_num_members) = guessed_num_members { - guessed_num_members - } else { - self.members(RoomMemberships::ACTIVE).await?.len() - }; - - // joined but the summary is not completed yet - (num_members as u64, summary.invited_member_count) - } - - _ => (summary.joined_member_count, summary.invited_member_count), - }; - - debug!( - room_id = ?self.room_id(), - own_user = ?self.own_user_id, - num_joined, num_invited, - heroes = ?heroes, - "Calculating name for a room", - ); - - Ok(self.inner.read().base_info.calculate_room_name( - num_joined, - num_invited, - heroes.iter().map(|hero| hero.as_str()).collect(), - )) - } - /// Subscribe to the inner `RoomInfo`. pub fn subscribe_info(&self) -> Subscriber { self.inner.subscribe() @@ -897,6 +919,13 @@ pub struct RoomInfo { /// spamming about unknown room versions in the log for the same room. #[serde(skip)] pub(crate) warned_about_unknown_room_version: Arc, + + /// Cached display name, useful for sync access. + /// + /// Filled by calling [`Self::compute_display_name`]. It's automatically + /// filled at start when creating a room, or on every successful sync. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) cached_display_name: Option, } #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -933,6 +962,7 @@ impl RoomInfo { read_receipts: Default::default(), base_info: Box::new(BaseRoomInfo::new()), warned_about_unknown_room_version: Arc::new(false.into()), + cached_display_name: None, } } @@ -1400,6 +1430,45 @@ impl RoomStateFilter { } } +/// Calculate room name according to step 3 of the [naming algorithm]. +/// +/// [naming algorithm]: https://spec.matrix.org/latest/client-server-api/#calculating-the-display-name-for-a-room +fn compute_display_name_from_heroes( + joined_member_count: u64, + invited_member_count: u64, + mut heroes: Vec<&str>, +) -> DisplayName { + let num_heroes = heroes.len() as u64; + let invited_joined = invited_member_count + joined_member_count; + let invited_joined_minus_one = invited_joined.saturating_sub(1); + + // Stabilize ordering. + heroes.sort_unstable(); + + let names = if num_heroes == 0 && invited_joined > 1 { + format!("{} people", invited_joined) + } else if num_heroes >= invited_joined_minus_one { + heroes.join(", ") + } else if num_heroes < invited_joined_minus_one && invited_joined > 1 { + // TODO: What length does the spec want us to use here and in + // the `else`? + format!("{}, and {} others", heroes.join(", "), (invited_joined - num_heroes)) + } else { + "".to_owned() + }; + + // User is alone. + if invited_joined <= 1 { + if names.is_empty() { + DisplayName::Empty + } else { + DisplayName::EmptyWas(names) + } + } else { + DisplayName::Calculated(names) + } +} + #[cfg(test)] mod tests { use std::{ @@ -1439,7 +1508,7 @@ mod tests { #[cfg(feature = "experimental-sliding-sync")] use super::SyncInfo; - use super::{Room, RoomInfo, RoomState}; + use super::{compute_display_name_from_heroes, Room, RoomInfo, RoomState}; #[cfg(any(feature = "experimental-sliding-sync", feature = "e2e-encryption"))] use crate::latest_event::LatestEvent; use crate::{ @@ -1481,6 +1550,7 @@ mod tests { base_info: Box::new(BaseRoomInfo::new()), read_receipts: Default::default(), warned_about_unknown_room_version: Arc::new(false.into()), + cached_display_name: None, }; let info_json = json!({ @@ -1528,7 +1598,7 @@ mod tests { "num_notifications": 0, "latest_active": null, "pending": [] - } + }, }); assert_eq!(serde_json::to_value(info).unwrap(), info_json); @@ -1574,7 +1644,8 @@ mod tests { "name": null, "tombstone": null, "topic": null, - } + }, + "cached_display_name": { "Calculated": "lol" } }); let info: RoomInfo = serde_json::from_value(info_json).unwrap(); @@ -1604,6 +1675,11 @@ mod tests { assert!(info.base_info.name.is_none()); assert!(info.base_info.tombstone.is_none()); assert!(info.base_info.topic.is_none()); + + assert_eq!( + info.cached_display_name.as_ref(), + Some(&DisplayName::Calculated("lol".to_owned())) + ) } #[async_test] @@ -1754,7 +1830,7 @@ mod tests { assert!(room.is_low_priority().not()); } - fn make_room(room_type: RoomState) -> (Arc, Room) { + fn make_room_test_helper(room_type: RoomState) -> (Arc, Room) { let store = Arc::new(MemoryStore::new()); let user_id = user_id!("@me:example.org"); let room_id = room_id!("!test:localhost"); @@ -1793,47 +1869,47 @@ mod tests { #[async_test] async fn test_display_name_for_joined_room_is_empty_if_no_info() { - let (_, room) = make_room(RoomState::Joined); - assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty); + let (_, room) = make_room_test_helper(RoomState::Joined); + assert_eq!(room.compute_display_name().await.unwrap(), DisplayName::Empty); } #[async_test] async fn test_display_name_for_joined_room_uses_canonical_alias_if_available() { - let (_, room) = make_room(RoomState::Joined); + let (_, room) = make_room_test_helper(RoomState::Joined); room.inner .update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event())); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()) ); } #[async_test] async fn test_display_name_for_joined_room_prefers_name_over_alias() { - let (_, room) = make_room(RoomState::Joined); + let (_, room) = make_room_test_helper(RoomState::Joined); room.inner .update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event())); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()) ); room.inner.update(|info| info.base_info.name = Some(make_name_event())); // Display name wasn't cached when we asked for it above, and name overrides assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Named("Test Room".to_owned()) ); } #[async_test] async fn test_display_name_for_invited_room_is_empty_if_no_info() { - let (_, room) = make_room(RoomState::Invited); - assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty); + let (_, room) = make_room_test_helper(RoomState::Invited); + assert_eq!(room.compute_display_name().await.unwrap(), DisplayName::Empty); } #[async_test] async fn test_display_name_for_invited_room_is_empty_if_room_name_empty() { - let (_, room) = make_room(RoomState::Invited); + let (_, room) = make_room_test_helper(RoomState::Invited); let room_name = MinimalStateEvent::Original(OriginalMinimalStateEvent { content: RoomNameEventContent::new(String::new()), @@ -1841,33 +1917,33 @@ mod tests { }); room.inner.update(|info| info.base_info.name = Some(room_name)); - assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty); + assert_eq!(room.compute_display_name().await.unwrap(), DisplayName::Empty); } #[async_test] async fn test_display_name_for_invited_room_uses_canonical_alias_if_available() { - let (_, room) = make_room(RoomState::Invited); + let (_, room) = make_room_test_helper(RoomState::Invited); room.inner .update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event())); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()) ); } #[async_test] async fn test_display_name_for_invited_room_prefers_name_over_alias() { - let (_, room) = make_room(RoomState::Invited); + let (_, room) = make_room_test_helper(RoomState::Invited); room.inner .update(|info| info.base_info.canonical_alias = Some(make_canonical_alias_event())); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Aliased("test".to_owned()) ); room.inner.update(|info| info.base_info.name = Some(make_name_event())); // Display name wasn't cached when we asked for it above, and name overrides assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Named("Test Room".to_owned()) ); } @@ -1890,7 +1966,7 @@ mod tests { #[async_test] async fn test_display_name_dm_invited() { - let (store, room) = make_room(RoomState::Invited); + let (store, room) = make_room_test_helper(RoomState::Invited); let room_id = room_id!("!test:localhost"); let matthew = user_id!("@matthew:example.org"); let me = user_id!("@me:example.org"); @@ -1909,14 +1985,14 @@ mod tests { room.inner.update_if(|info| info.update_from_ruma_summary(&summary)); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Matthew".to_owned()) ); } #[async_test] async fn test_display_name_dm_invited_no_heroes() { - let (store, room) = make_room(RoomState::Invited); + let (store, room) = make_room_test_helper(RoomState::Invited); let room_id = room_id!("!test:localhost"); let matthew = user_id!("@matthew:example.org"); let me = user_id!("@me:example.org"); @@ -1931,14 +2007,14 @@ mod tests { store.save_changes(&changes).await.unwrap(); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Matthew".to_owned()) ); } #[async_test] async fn test_display_name_dm_joined() { - let (store, room) = make_room(RoomState::Joined); + let (store, room) = make_room_test_helper(RoomState::Joined); let room_id = room_id!("!test:localhost"); let matthew = user_id!("@matthew:example.org"); let me = user_id!("@me:example.org"); @@ -1961,14 +2037,14 @@ mod tests { room.inner.update_if(|info| info.update_from_ruma_summary(&summary)); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Matthew".to_owned()) ); } #[async_test] async fn test_display_name_dm_joined_no_heroes() { - let (store, room) = make_room(RoomState::Joined); + let (store, room) = make_room_test_helper(RoomState::Joined); let room_id = room_id!("!test:localhost"); let matthew = user_id!("@matthew:example.org"); let me = user_id!("@me:example.org"); @@ -1986,14 +2062,14 @@ mod tests { store.save_changes(&changes).await.unwrap(); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Matthew".to_owned()) ); } #[async_test] async fn test_display_name_deterministic() { - let (store, room) = make_room(RoomState::Joined); + let (store, room) = make_room_test_helper(RoomState::Joined); let alice = user_id!("@alice:example.org"); let bob = user_id!("@bob:example.org"); @@ -2041,14 +2117,14 @@ mod tests { room.inner.update_if(|info| info.update_from_ruma_summary(&summary)); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Bob, Carol, Denis, Erica, and 3 others".to_owned()) ); } #[async_test] async fn test_display_name_deterministic_no_heroes() { - let (store, room) = make_room(RoomState::Joined); + let (store, room) = make_room_test_helper(RoomState::Joined); let alice = user_id!("@alice:example.org"); let bob = user_id!("@bob:example.org"); @@ -2090,14 +2166,14 @@ mod tests { } assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::Calculated("Alice, Bob, Carol, Denis, Erica, and 2 others".to_owned()) ); } #[async_test] async fn test_display_name_dm_alone() { - let (store, room) = make_room(RoomState::Joined); + let (store, room) = make_room_test_helper(RoomState::Joined); let room_id = room_id!("!test:localhost"); let matthew = user_id!("@matthew:example.org"); let me = user_id!("@me:example.org"); @@ -2120,7 +2196,7 @@ mod tests { room.inner.update_if(|info| info.update_from_ruma_summary(&summary)); assert_eq!( - room.computed_display_name().await.unwrap(), + room.compute_display_name().await.unwrap(), DisplayName::EmptyWas("Matthew".to_owned()) ); } @@ -2173,11 +2249,11 @@ mod tests { assert_pending!(room_info_subscriber); } - #[test] + #[async_test] #[cfg(feature = "experimental-sliding-sync")] - fn test_when_we_provide_a_newly_decrypted_event_it_replaces_latest_event() { + async fn test_when_we_provide_a_newly_decrypted_event_it_replaces_latest_event() { // Given a room with an encrypted event - let (_store, room) = make_room(RoomState::Joined); + let (_store, room) = make_room_test_helper(RoomState::Joined); add_encrypted_event(&room, "$A"); // Sanity: it has no latest_event assert!(room.latest_event().is_none()); @@ -2192,11 +2268,11 @@ mod tests { assert_eq!(room.latest_event().unwrap().event_id(), event.event_id()); } - #[test] + #[async_test] #[cfg(feature = "experimental-sliding-sync")] - fn test_when_a_newly_decrypted_event_appears_we_delete_all_older_encrypted_events() { + async fn test_when_a_newly_decrypted_event_appears_we_delete_all_older_encrypted_events() { // Given a room with some encrypted events and a latest event - let (_store, room) = make_room(RoomState::Joined); + let (_store, room) = make_room_test_helper(RoomState::Joined); room.inner.update(|info| info.latest_event = Some(make_latest_event("$A"))); add_encrypted_event(&room, "$0"); add_encrypted_event(&room, "$1"); @@ -2220,11 +2296,11 @@ mod tests { assert_eq!(room.latest_event().unwrap().event_id(), new_event.event_id()); } - #[test] + #[async_test] #[cfg(feature = "experimental-sliding-sync")] - fn test_replacing_the_newest_event_leaves_none_left() { + async fn test_replacing_the_newest_event_leaves_none_left() { // Given a room with some encrypted events - let (_store, room) = make_room(RoomState::Joined); + let (_store, room) = make_room_test_helper(RoomState::Joined); add_encrypted_event(&room, "$0"); add_encrypted_event(&room, "$1"); add_encrypted_event(&room, "$2"); @@ -2323,7 +2399,7 @@ mod tests { /// `user_b`: one membership /// `user_c`: two memberships (two devices) fn create_call_with_member_events_for_user(a: &UserId, b: &UserId, c: &UserId) -> Room { - let (_, room) = make_room(RoomState::Joined); + let (_, room) = make_room_test_helper(RoomState::Joined); let a_empty = call_member_state_event(Vec::new(), "$1234", a); @@ -2370,4 +2446,40 @@ mod tests { assert_eq!(Vec::::new(), room.active_room_call_participants()); assert!(!room.has_active_room_call()); } + + #[test] + fn test_calculate_room_name() { + let mut actual = compute_display_name_from_heroes(2, 0, vec!["a"]); + assert_eq!(DisplayName::Calculated("a".to_owned()), actual); + + actual = compute_display_name_from_heroes(3, 0, vec!["a", "b"]); + assert_eq!(DisplayName::Calculated("a, b".to_owned()), actual); + + actual = compute_display_name_from_heroes(4, 0, vec!["a", "b", "c"]); + assert_eq!(DisplayName::Calculated("a, b, c".to_owned()), actual); + + actual = compute_display_name_from_heroes(5, 0, vec!["a", "b", "c"]); + assert_eq!(DisplayName::Calculated("a, b, c, and 2 others".to_owned()), actual); + + actual = compute_display_name_from_heroes(5, 0, vec![]); + assert_eq!(DisplayName::Calculated("5 people".to_owned()), actual); + + actual = compute_display_name_from_heroes(0, 0, vec![]); + assert_eq!(DisplayName::Empty, actual); + + actual = compute_display_name_from_heroes(1, 0, vec![]); + assert_eq!(DisplayName::Empty, actual); + + actual = compute_display_name_from_heroes(0, 1, vec![]); + assert_eq!(DisplayName::Empty, actual); + + actual = compute_display_name_from_heroes(1, 0, vec!["a"]); + assert_eq!(DisplayName::EmptyWas("a".to_owned()), actual); + + actual = compute_display_name_from_heroes(1, 0, vec!["a", "b"]); + assert_eq!(DisplayName::EmptyWas("a, b".to_owned()), actual); + + actual = compute_display_name_from_heroes(1, 0, vec!["a", "b", "c"]); + assert_eq!(DisplayName::EmptyWas("a, b, c".to_owned()), actual); + } } diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 5df2068e48f..939d3cbddb3 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -295,6 +295,13 @@ impl BaseClient { self.apply_changes(&changes, false); trace!("applied changes"); + // Now that all the rooms information have been saved, update the display name + // cache (which relies on information stored in the database). This will + // live in memory, until the next sync which will saves the room info to + // disk; we do this to avoid saving that would be redundant with the + // above. Oh well. + new_rooms.update_in_memory_caches(&self.store).await; + Ok(SyncResponse { rooms: new_rooms, notifications, @@ -738,6 +745,7 @@ mod tests { canonical_alias::RoomCanonicalAliasEventContent, member::{MembershipState, RoomMemberEventContent}, message::SyncRoomMessageEvent, + name::RoomNameEventContent, }, AnySyncMessageLikeEvent, AnySyncTimelineEvent, GlobalAccountDataEventContent, StateEventContent, @@ -817,22 +825,23 @@ mod tests { } #[async_test] - async fn test_room_name_is_found_when_processing_sliding_sync_response() { + async fn test_missing_room_name_event() { // Given a logged-in client let client = logged_in_base_client(None).await; let room_id = room_id!("!r:e.uk"); - // When I send sliding sync response containing a room with a name + // When I send sliding sync response containing a room with a name set in the + // sliding sync response, let mut room = v4::SlidingSyncRoom::new(); room.name = Some("little room".to_owned()); let response = response_with_room(room_id, room); let sync_resp = client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); - // Then the room doesn't have the name in the client. + // No m.room.name event, no heroes, no members => considered an empty room! let client_room = client.get_room(room_id).expect("No room found"); assert!(client_room.name().is_none()); - assert_eq!(client_room.computed_display_name().await.unwrap().to_string(), "Empty Room"); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "Empty Room"); assert_eq!(client_room.state(), RoomState::Joined); // And it is added to the list of joined rooms only. @@ -842,14 +851,37 @@ mod tests { } #[async_test] - async fn test_invited_room_name_is_found_when_processing_sliding_sync_response() { + async fn test_room_name_event() { + // Given a logged-in client + let client = logged_in_base_client(None).await; + let room_id = room_id!("!r:e.uk"); + + // When I send sliding sync response containing a room with a name set in the + // sliding sync response, and a m.room.name event, + let mut room = v4::SlidingSyncRoom::new(); + + room.name = Some("little room".to_owned()); + set_room_name(&mut room, user_id!("@a:b.c"), "The Name".to_owned()); + + let response = response_with_room(room_id, room); + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + // The name is known. + let client_room = client.get_room(room_id).expect("No room found"); + assert_eq!(client_room.name().as_deref(), Some("The Name")); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "The Name"); + } + + #[async_test] + async fn test_missing_invited_room_name_event() { // Given a logged-in client, let client = logged_in_base_client(None).await; let room_id = room_id!("!r:e.uk"); let user_id = user_id!("@w:e.uk"); let inviter = user_id!("@john:mastodon.org"); - // When I send sliding sync response containing a room with a name, + // When I send sliding sync response containing a room with a name set in the + // sliding sync response, let mut room = v4::SlidingSyncRoom::new(); set_room_invited(&mut room, inviter, user_id); room.name = Some("name from sliding sync response".to_owned()); @@ -861,9 +893,8 @@ mod tests { let client_room = client.get_room(room_id).expect("No room found"); assert!(client_room.name().is_none()); - // The computed display name will show heroes name, but for an invited room… - // it's only self, the invitee, who will appear. - assert_eq!(client_room.computed_display_name().await.unwrap().to_string(), "w"); + // No m.room.name event, no heroes, no members => considered an empty room! + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "Empty Room"); assert_eq!(client_room.state(), RoomState::Invited); @@ -873,6 +904,32 @@ mod tests { assert!(sync_resp.rooms.invite.contains_key(room_id)); } + #[async_test] + async fn test_invited_room_name_event() { + // Given a logged-in client, + let client = logged_in_base_client(None).await; + let room_id = room_id!("!r:e.uk"); + let user_id = user_id!("@w:e.uk"); + let inviter = user_id!("@john:mastodon.org"); + + // When I send sliding sync response containing a room with a name set in the + // sliding sync response, and a m.room.name event, + let mut room = v4::SlidingSyncRoom::new(); + + set_room_invited(&mut room, inviter, user_id); + + room.name = Some("name from sliding sync response".to_owned()); + set_room_name(&mut room, user_id!("@a:b.c"), "The Name".to_owned()); + + let response = response_with_room(room_id, room); + client.process_sliding_sync(&response, &()).await.expect("Failed to process sync"); + + // The name is known. + let client_room = client.get_room(room_id).expect("No room found"); + assert_eq!(client_room.name().as_deref(), Some("The Name")); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "The Name"); + } + #[async_test] async fn test_left_a_room_from_required_state_event() { // Given a logged-in client @@ -1278,7 +1335,7 @@ mod tests { // Then the room's name is NOT overridden by the server-computed display name. let client_room = client.get_room(room_id).expect("No room found"); - assert_eq!(client_room.computed_display_name().await.unwrap().to_string(), "myroom"); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "myroom"); assert!(client_room.name().is_none()); } @@ -1857,6 +1914,15 @@ mod tests { room } + fn set_room_name(room: &mut v4::SlidingSyncRoom, sender: &UserId, name: String) { + room.required_state.push(make_state_event( + sender, + "", + RoomNameEventContent::new(name), + None, + )); + } + fn set_room_invited(room: &mut v4::SlidingSyncRoom, inviter: &UserId, invitee: &UserId) { // MSC3575 shows an almost-empty event to indicate that we are invited to a // room. Just the type is supplied. diff --git a/crates/matrix-sdk-base/src/store/migration_helpers.rs b/crates/matrix-sdk-base/src/store/migration_helpers.rs index 79c991f8828..9621afce82d 100644 --- a/crates/matrix-sdk-base/src/store/migration_helpers.rs +++ b/crates/matrix-sdk-base/src/store/migration_helpers.rs @@ -124,6 +124,7 @@ impl RoomInfoV1 { read_receipts: Default::default(), base_info: base_info.migrate(create), warned_about_unknown_room_version: Arc::new(false.into()), + cached_display_name: None, } } } diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index c812770aaed..81e2eef8a3d 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -183,6 +183,7 @@ impl Store { info, roominfo_update_sender.clone(), ); + self.rooms.write().unwrap().insert(room.room_id().to_owned(), room); } @@ -222,7 +223,7 @@ impl Store { } /// Lookup the Room for the given RoomId, or create one, if it didn't exist - /// yet in the store + /// yet in the store. pub fn get_or_create_room( &self, room_id: &RoomId, diff --git a/crates/matrix-sdk-base/src/sync.rs b/crates/matrix-sdk-base/src/sync.rs index 23f1bff59a4..50cc6210b19 100644 --- a/crates/matrix-sdk-base/src/sync.rs +++ b/crates/matrix-sdk-base/src/sync.rs @@ -35,6 +35,7 @@ use serde::{Deserialize, Serialize}; use crate::{ debug::{DebugInvitedRoom, DebugListOfRawEvents, DebugListOfRawEventsNoId}, deserialized_responses::{AmbiguityChange, RawAnySyncOrStrippedTimelineEvent}, + store::Store, }; /// Generalized representation of a `/sync` response. @@ -78,6 +79,23 @@ pub struct RoomUpdates { pub invite: BTreeMap, } +impl RoomUpdates { + /// Update the caches for the rooms that received updates. + /// + /// This will only fill the in-memory caches, not save the info on disk. + pub(crate) async fn update_in_memory_caches(&self, store: &Store) { + for room in self + .leave + .keys() + .chain(self.join.keys()) + .chain(self.invite.keys()) + .filter_map(|room_id| store.get_room(room_id)) + { + let _ = room.compute_display_name().await; + } + } +} + #[cfg(not(tarpaulin_include))] impl fmt::Debug for RoomUpdates { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/crates/matrix-sdk-ui/src/notification_client.rs b/crates/matrix-sdk-ui/src/notification_client.rs index 6a110439c08..64f475106e4 100644 --- a/crates/matrix-sdk-ui/src/notification_client.rs +++ b/crates/matrix-sdk-ui/src/notification_client.rs @@ -668,7 +668,7 @@ impl NotificationItem { sender_display_name, sender_avatar_url, is_sender_name_ambiguous, - room_computed_display_name: room.computed_display_name().await?.to_string(), + room_computed_display_name: room.compute_display_name().await?.to_string(), room_avatar_url: room.avatar_url().map(|s| s.to_string()), room_canonical_alias: room.canonical_alias().map(|c| c.to_string()), is_direct_message_room: room.is_direct().await?, diff --git a/crates/matrix-sdk-ui/src/room_list_service/filters/fuzzy_match_room_name.rs b/crates/matrix-sdk-ui/src/room_list_service/filters/fuzzy_match_room_name.rs index 4b476035fc6..bd127f9f5c9 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/filters/fuzzy_match_room_name.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/filters/fuzzy_match_room_name.rs @@ -53,9 +53,9 @@ pub fn new_filter(client: &Client, pattern: &str) -> impl Filter { move |room_list_entry| -> bool { let Some(room_id) = room_list_entry.as_room_id() else { return false }; let Some(room) = client.get_room(room_id) else { return false }; - let Some(room_name) = room.name() else { return false }; + let Some(room_name) = room.cached_display_name() else { return false }; - searcher.matches(&room_name) + searcher.matches(&room_name.to_string()) } } diff --git a/crates/matrix-sdk-ui/src/room_list_service/filters/normalized_match_room_name.rs b/crates/matrix-sdk-ui/src/room_list_service/filters/normalized_match_room_name.rs index 184404bd78e..55c510b5564 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/filters/normalized_match_room_name.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/filters/normalized_match_room_name.rs @@ -53,9 +53,9 @@ pub fn new_filter(client: &Client, pattern: &str) -> impl Filter { move |room_list_entry| -> bool { let Some(room_id) = room_list_entry.as_room_id() else { return false }; let Some(room) = client.get_room(room_id) else { return false }; - let Some(room_name) = room.name() else { return false }; + let Some(room_name) = room.cached_display_name() else { return false }; - searcher.matches(&room_name) + searcher.matches(&room_name.to_string()) } } diff --git a/crates/matrix-sdk-ui/src/room_list_service/room.rs b/crates/matrix-sdk-ui/src/room_list_service/room.rs index 667ed1fde9d..94b7159fea2 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room.rs @@ -84,8 +84,8 @@ impl Room { } /// Get a computed room name for the room. - pub async fn computed_display_name(&self) -> Option { - Some(self.inner.room.computed_display_name().await.ok()?.to_string()) + pub fn cached_display_name(&self) -> Option { + Some(self.inner.room.cached_display_name()?.to_string()) } /// Get the underlying [`matrix_sdk::Room`]. diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index 0821b0f5bae..4237257615a 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -2075,7 +2075,7 @@ async fn test_room() -> Result<(), Error> { let room0 = room_list.room(room_id_0).await?; // Room has received a name from sliding sync. - assert_eq!(room0.computed_display_name().await, Some("Room #0".to_owned())); + assert_eq!(room0.cached_display_name(), Some("Room #0".to_owned())); // Room has received an avatar from sliding sync. assert_eq!(room0.avatar_url(), Some(mxc_uri!("mxc://homeserver/media").to_owned())); @@ -2083,7 +2083,7 @@ async fn test_room() -> Result<(), Error> { let room1 = room_list.room(room_id_1).await?; // Room has not received a name from sliding sync, then it's calculated. - assert_eq!(room1.computed_display_name().await, Some("Empty Room".to_owned())); + assert_eq!(room1.cached_display_name(), Some("Empty Room".to_owned())); // Room has not received an avatar from sliding sync, then it's calculated, but // there is nothing to calculate from, so there is no URL. @@ -2129,7 +2129,7 @@ async fn test_room() -> Result<(), Error> { }; // Room has _now_ received a name from sliding sync! - assert_eq!(room1.computed_display_name().await, Some("Room #1".to_owned())); + assert_eq!(room1.cached_display_name(), Some("Room #1".to_owned())); // Room has _now_ received an avatar URL from sliding sync! assert_eq!(room1.avatar_url(), Some(mxc_uri!("mxc://homeserver/other-media").to_owned())); diff --git a/crates/matrix-sdk/tests/integration/room/common.rs b/crates/matrix-sdk/tests/integration/room/common.rs index a3728130736..c01c0b7a17e 100644 --- a/crates/matrix-sdk/tests/integration/room/common.rs +++ b/crates/matrix-sdk/tests/integration/room/common.rs @@ -58,7 +58,7 @@ async fn test_calculate_room_names_from_summary() { assert_eq!( DisplayName::Calculated("example2".to_owned()), - room.computed_display_name().await.unwrap() + room.compute_display_name().await.unwrap() ); } @@ -77,7 +77,7 @@ async fn test_room_names() { assert_eq!( DisplayName::Aliased("tutorial".to_owned()), - room.computed_display_name().await.unwrap() + room.compute_display_name().await.unwrap() ); mock_sync(&server, &*test_json::INVITE_SYNC, Some(sync_token.clone())).await; @@ -89,7 +89,7 @@ async fn test_room_names() { assert_eq!( DisplayName::Named("My Room Name".to_owned()), - invited_room.computed_display_name().await.unwrap() + invited_room.compute_display_name().await.unwrap() ); } diff --git a/examples/oidc_cli/src/main.rs b/examples/oidc_cli/src/main.rs index 20794444b1d..55d0d16c0c5 100644 --- a/examples/oidc_cli/src/main.rs +++ b/examples/oidc_cli/src/main.rs @@ -881,7 +881,7 @@ async fn on_room_message(event: OriginalSyncRoomMessageEvent, room: Room) { } let MessageType::Text(text_content) = &event.content.msgtype else { return }; - let room_name = match room.computed_display_name().await { + let room_name = match room.compute_display_name().await { Ok(room_name) => room_name.to_string(), Err(error) => { println!("Error getting room display name: {error}"); diff --git a/examples/persist_session/src/main.rs b/examples/persist_session/src/main.rs index 6b880f36502..3ce375dd8b2 100644 --- a/examples/persist_session/src/main.rs +++ b/examples/persist_session/src/main.rs @@ -297,7 +297,7 @@ async fn on_room_message(event: OriginalSyncRoomMessageEvent, room: Room) { } let MessageType::Text(text_content) = &event.content.msgtype else { return }; - let room_name = match room.computed_display_name().await { + let room_name = match room.compute_display_name().await { Ok(room_name) => room_name.to_string(), Err(error) => { println!("Error getting room display name: {error}"); diff --git a/labs/multiverse/src/main.rs b/labs/multiverse/src/main.rs index d3b56442db4..775b97651c8 100644 --- a/labs/multiverse/src/main.rs +++ b/labs/multiverse/src/main.rs @@ -272,7 +272,7 @@ impl App { for (room_id, room) in &new_ui_rooms { let raw_name = room.name(); - let display_name = room.computed_display_name().await; + let display_name = room.cached_display_name(); room_infos .lock() .unwrap() 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 d5d537d0ef1..5886a0db184 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 @@ -4,7 +4,6 @@ use std::{ }; use anyhow::Result; -use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::{pin_mut, FutureExt, StreamExt as _}; use matrix_sdk::{ @@ -355,9 +354,12 @@ impl UpdateObserver { /// /// Returns `None` when the diff was empty, aka it was a spurious update. async fn next(&mut self) -> Option { - assert_let!( - Ok(Some(update)) = timeout(Duration::from_secs(3), self.subscriber.next()).await - ); + // Wait for the room info updates to stabilize. + let mut update = None; + while let Ok(Some(up)) = timeout(Duration::from_secs(2), self.subscriber.next()).await { + update = Some(up); + } + let update = update.expect("there should have been an update"); let update_json = serde_json::to_value(&update).unwrap(); let update_diff = json_structural_diff::JsonDiff::diff_string( @@ -570,12 +572,7 @@ async fn test_room_notification_count() -> Result<()> { { debug!("Remote echo of marking the room as read"); - let update = update_observer.next().await; - - if update.is_none() { - debug!("Previous update was spurious, actual update now"); - update_observer.next().await.expect("there should be a non-empty update at some point"); - } + update_observer.next().await; assert!(!alice_room.are_members_synced()); @@ -600,7 +597,7 @@ async fn test_room_notification_count() -> Result<()> { alice_room.send(RoomMessageEventContent::text_plain("hello bob")).await?; { - debug!("Room members got synced."); + debug!("Room members got synced + remote echo for hello bob."); update_observer.next().await.expect("syncing room members should update room info"); assert!(alice_room.are_members_synced()); @@ -612,17 +609,6 @@ async fn test_room_notification_count() -> Result<()> { update_observer.assert_is_pending(); } - { - debug!("Remote echo for hello bob"); - update_observer.next().await.expect("we should receive a remote echo for our own message"); - - assert_eq!(alice_room.num_unread_messages(), 0); - assert_eq!(alice_room.num_unread_notifications(), 0); - assert_eq!(alice_room.num_unread_mentions(), 0); - - update_observer.assert_is_pending(); - } - // Now Alice is only interesting in mentions of their name. let settings = alice.notification_settings().await;