Skip to content

Commit

Permalink
base: rename computed_display_name to compute_display_name and remove…
Browse files Browse the repository at this point in the history
… computed_ in the cached one
  • Loading branch information
bnjbvr committed Jun 6, 2024
1 parent 9021c18 commit 823226a
Show file tree
Hide file tree
Showing 13 changed files with 49 additions and 47 deletions.
2 changes: 1 addition & 1 deletion bindings/matrix-sdk-ffi/src/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> {
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.
Expand Down
2 changes: 1 addition & 1 deletion bindings/matrix-sdk-ffi/src/room_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
);
}
Expand Down
60 changes: 31 additions & 29 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,11 @@ pub struct Room {
#[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))]
pub latest_encrypted_events: Arc<SyncRwLock<RingBuffer<Raw<AnySyncTimelineEvent>>>>,

/// 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<SyncRwLock<Option<DisplayName>>>,
/// 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<SyncRwLock<Option<DisplayName>>>,
}

/// The room summary containing member counts and members that should be used to
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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]: <https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room>
pub async fn computed_display_name(&self) -> StoreResult<DisplayName> {
pub async fn compute_display_name(&self) -> StoreResult<DisplayName> {
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
};

Expand Down Expand Up @@ -592,8 +591,11 @@ impl Room {
}

/// Returns the cached computed display name, if available.
pub fn cached_computed_display_name(&self) -> Option<DisplayName> {
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<DisplayName> {
self.cached_display_name.read().unwrap().clone()
}

/// Return the last event in this room, if one has been cached during
Expand Down Expand Up @@ -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]
Expand All @@ -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())
);
}
Expand All @@ -1872,21 +1874,21 @@ 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())
);
}

#[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]
Expand All @@ -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]
Expand All @@ -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())
);
}
Expand All @@ -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())
);
}
Expand Down Expand Up @@ -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())
);
}
Expand All @@ -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())
);
}
Expand Down Expand Up @@ -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())
);
}
Expand All @@ -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())
);
}
Expand Down Expand Up @@ -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())
);
}
Expand Down Expand Up @@ -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())
);
}
Expand Down Expand Up @@ -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())
);
}
Expand Down
10 changes: 5 additions & 5 deletions crates/matrix-sdk-base/src/sliding_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand All @@ -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);

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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());
}

Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-ui/src/notification_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk-ui/src/room_list_service/room.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl Room {

/// Get a computed room name for the room.
pub fn cached_display_name(&self) -> Option<String> {
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`].
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
6 changes: 3 additions & 3 deletions crates/matrix-sdk/tests/integration/room/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand All @@ -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;
Expand All @@ -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()
);
}

Expand Down
2 changes: 1 addition & 1 deletion examples/oidc_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Expand Down
2 changes: 1 addition & 1 deletion examples/persist_session/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Expand Down

0 comments on commit 823226a

Please sign in to comment.