From 823226a3cdbe2f4c83151ecc99fcc9e4f0521c22 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Thu, 6 Jun 2024 12:23:28 +0200 Subject: [PATCH] base: rename computed_display_name to compute_display_name and remove computed_ in the cached one --- bindings/matrix-sdk-ffi/src/room.rs | 2 +- bindings/matrix-sdk-ffi/src/room_info.rs | 2 +- crates/matrix-sdk-base/src/client.rs | 2 +- crates/matrix-sdk-base/src/rooms/normal.rs | 60 ++++++++++--------- crates/matrix-sdk-base/src/sliding_sync.rs | 10 ++-- .../matrix-sdk-ui/src/notification_client.rs | 2 +- .../filters/fuzzy_match_room_name.rs | 2 +- .../filters/normalized_match_room_name.rs | 2 +- .../src/room_list_service/room.rs | 2 +- crates/matrix-sdk/src/sync.rs | 2 +- .../tests/integration/room/common.rs | 6 +- examples/oidc_cli/src/main.rs | 2 +- examples/persist_session/src/main.rs | 2 +- 13 files changed, 49 insertions(+), 47 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 2d3ae4c4cb1..a31d35062b7 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -82,7 +82,7 @@ impl Room { /// compute a room name based on the room's nature (DM or not) and number of /// members. pub fn display_name(&self) -> Option { - Some(self.inner.cached_computed_display_name()?.to_string()) + 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 64cbedbc347..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.cached_computed_display_name().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/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 7202afcc1e3..77b9b4f9b79 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -1660,7 +1660,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/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index c75c325f163..3b37d2d6772 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -103,12 +103,11 @@ pub struct Room { #[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))] pub latest_encrypted_events: Arc>>>, - /// Cached value of the last call to `computed_display_name()`, useful for - /// sync access. + /// Cached display name, useful for sync access. /// - /// Filled by calling `computed_display_name`. It's automatically filled at - /// start, when creating a room, or on every successful sync. - pub(crate) cached_computed_display_name: Arc>>, + /// Filled by calling [`Self::compute_display_name`]. It's automatically + /// filled at start when creating a room, or on every successful sync. + pub(crate) cached_display_name: Arc>>, } /// The room summary containing member counts and members that should be used to @@ -207,11 +206,11 @@ impl Room { Self::MAX_ENCRYPTED_EVENTS, ))), roominfo_update_sender, - cached_computed_display_name: Arc::new(SyncRwLock::new(None)), + cached_display_name: Arc::new(SyncRwLock::new(None)), }; - // Refill the computed_display_name cache. - let _ = room.computed_display_name().await; + // Refill the display name cache. + let _ = room.compute_display_name().await; room } @@ -491,12 +490,12 @@ impl Room { /// /// This is automatically recomputed on every successful sync, and the /// cached result can be retrieved in - /// [`Self::cached_computed_display_name`]. + /// [`Self::cached_display_name`]. /// /// [spec]: - pub async fn computed_display_name(&self) -> StoreResult { + pub async fn compute_display_name(&self) -> StoreResult { let update_cache = |new_val: DisplayName| { - *self.cached_computed_display_name.write().unwrap() = Some(new_val.clone()); + *self.cached_display_name.write().unwrap() = Some(new_val.clone()); new_val }; @@ -592,8 +591,11 @@ impl Room { } /// Returns the cached computed display name, if available. - pub fn cached_computed_display_name(&self) -> Option { - self.cached_computed_display_name.read().unwrap().clone() + /// + /// This cache is refilled every time we call + /// [`Self::compute_display_name`]. + pub fn cached_display_name(&self) -> Option { + self.cached_display_name.read().unwrap().clone() } /// Return the last event in this room, if one has been cached during @@ -1852,7 +1854,7 @@ mod tests { #[async_test] async fn test_display_name_for_joined_room_is_empty_if_no_info() { let (_, room) = make_room_test_helper(RoomState::Joined).await; - assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty); + assert_eq!(room.compute_display_name().await.unwrap(), DisplayName::Empty); } #[async_test] @@ -1861,7 +1863,7 @@ mod tests { 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()) ); } @@ -1872,13 +1874,13 @@ mod tests { 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()) ); } @@ -1886,7 +1888,7 @@ mod tests { #[async_test] async fn test_display_name_for_invited_room_is_empty_if_no_info() { let (_, room) = make_room_test_helper(RoomState::Invited).await; - assert_eq!(room.computed_display_name().await.unwrap(), DisplayName::Empty); + assert_eq!(room.compute_display_name().await.unwrap(), DisplayName::Empty); } #[async_test] @@ -1899,7 +1901,7 @@ 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] @@ -1908,7 +1910,7 @@ mod tests { 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()) ); } @@ -1919,13 +1921,13 @@ mod tests { 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()) ); } @@ -1967,7 +1969,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::Calculated("Matthew".to_owned()) ); } @@ -1989,7 +1991,7 @@ 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()) ); } @@ -2019,7 +2021,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::Calculated("Matthew".to_owned()) ); } @@ -2044,7 +2046,7 @@ 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()) ); } @@ -2099,7 +2101,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::Calculated("Bob, Carol, Denis, Erica, and 3 others".to_owned()) ); } @@ -2148,7 +2150,7 @@ 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()) ); } @@ -2178,7 +2180,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()) ); } diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 7ce1b94b620..b4eea2d8b44 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -835,7 +835,7 @@ mod tests { // 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. @@ -863,7 +863,7 @@ mod tests { // 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.computed_display_name().await.unwrap().to_string(), "The Name"); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "The Name"); } #[async_test] @@ -888,7 +888,7 @@ mod tests { assert!(client_room.name().is_none()); // No m.room.name event, no heroes, no members => considered an empty room! - 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::Invited); @@ -921,7 +921,7 @@ mod tests { // 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.computed_display_name().await.unwrap().to_string(), "The Name"); + assert_eq!(client_room.compute_display_name().await.unwrap().to_string(), "The Name"); } #[async_test] @@ -1329,7 +1329,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()); } 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 196f0bbc06e..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,7 +53,7 @@ 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.cached_computed_display_name() else { return false }; + let Some(room_name) = room.cached_display_name() else { return false }; 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 05a3ec6a163..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,7 +53,7 @@ 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.cached_computed_display_name() else { return false }; + let Some(room_name) = room.cached_display_name() else { return false }; 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 a2acec1075f..94b7159fea2 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room.rs @@ -85,7 +85,7 @@ impl Room { /// Get a computed room name for the room. pub fn cached_display_name(&self) -> Option { - Some(self.inner.room.cached_computed_display_name()?.to_string()) + Some(self.inner.room.cached_display_name()?.to_string()) } /// Get the underlying [`matrix_sdk::Room`]. diff --git a/crates/matrix-sdk/src/sync.rs b/crates/matrix-sdk/src/sync.rs index 9ca4d760427..8c3ecec3473 100644 --- a/crates/matrix-sdk/src/sync.rs +++ b/crates/matrix-sdk/src/sync.rs @@ -163,7 +163,7 @@ impl Client { .chain(rooms.invite.keys()) .filter_map(|room_id| self.get_room(room_id)) { - let _ = room.computed_display_name().await; + let _ = room.compute_display_name().await; } } 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}");