diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 64e4b59f2fa..d80da192610 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -573,63 +573,61 @@ impl Room { /// /// [spec]: pub async fn compute_display_name(&self) -> StoreResult { - let update_cache = |new_val: RoomDisplayName| { - 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 - }; + enum DisplayNameOrSummary { + Summary(RoomSummary), + DisplayName(RoomDisplayName), + } - let summary = { + let display_name_or_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(RoomDisplayName::Named(name))); + match (inner.name(), inner.canonical_alias()) { + (Some(name), _) => { + let name = RoomDisplayName::Named(name.trim().to_owned()); + DisplayNameOrSummary::DisplayName(name) + } + (None, Some(alias)) => { + let name = RoomDisplayName::Aliased(alias.alias().trim().to_owned()); + DisplayNameOrSummary::DisplayName(name) + } + // We can't directly compute the display name from the summary here because Rust + // thinks that the `inner` lock is still held even if we explicitly call `drop()` + // on it. So we introduced the DisplayNameOrSummary type and do the computation in + // two steps. + (None, None) => DisplayNameOrSummary::Summary(inner.summary.clone()), } + }; - if let Some(alias) = inner.canonical_alias() { - let alias = alias.alias().trim().to_owned(); - drop(inner); // See above comment. - return Ok(update_cache(RoomDisplayName::Aliased(alias))); + let display_name = match display_name_or_summary { + DisplayNameOrSummary::Summary(summary) => { + self.compute_display_name_from_summary(summary).await? } - - inner.summary.clone() + DisplayNameOrSummary::DisplayName(display_name) => display_name, }; - // 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_invited_guess) = if !summary.room_heroes.is_empty() { - let mut names = Vec::with_capacity(summary.room_heroes.len()); - for hero in &summary.room_heroes { - if hero.user_id == own_user_id { - continue; - } - if let Some(display_name) = &hero.display_name { - names.push(display_name.clone()); - continue; - } - match self.get_member(&hero.user_id).await { - Ok(Some(member)) => { - names.push(member.name().to_owned()); - } - Ok(None) => { - warn!("Ignoring hero, no member info for {}", hero.user_id); - } - Err(error) => { - warn!("Ignoring hero, error getting member: {}", error); - } - } + // Update the cached display name before we return the newly computed value. + self.inner.update_if(|info| { + if info.cached_display_name.as_ref() != Some(&display_name) { + info.cached_display_name = Some(display_name.clone()); + true + } else { + false } + }); - (names, None) + Ok(display_name) + } + + /// Compute a [`RoomDisplayName`] from the given [`RoomSummary`]. + async fn compute_display_name_from_summary( + &self, + summary: RoomSummary, + ) -> StoreResult { + let summary_member_count = summary.joined_member_count + summary.invited_member_count; + + let (heroes, num_joined_invited_guess) = if !summary.room_heroes.is_empty() { + let heroes = self.extract_heroes(&summary.room_heroes).await?; + (heroes, None) } else { let (heroes, num_joined_invited) = self.compute_summary().await?; (heroes, Some(num_joined_invited)) @@ -639,7 +637,7 @@ impl Room { // when we were invited we don't have a proper summary, we have to do best // guessing heroes.len() as u64 + 1 - } else if summary.joined_member_count == 0 && summary.invited_member_count == 0 { + } else if summary_member_count == 0 { if let Some(num_joined_invited) = num_joined_invited_guess { num_joined_invited } else { @@ -649,7 +647,7 @@ impl Room { .len() as u64 } } else { - summary.joined_member_count + summary.invited_member_count + summary_member_count }; debug!( @@ -660,10 +658,43 @@ impl Room { "Calculating name for a room based on heroes", ); - Ok(update_cache(compute_display_name_from_heroes( + let display_name = compute_display_name_from_heroes( num_joined_invited, heroes.iter().map(|hero| hero.as_str()).collect(), - ))) + ); + + Ok(display_name) + } + + /// Extract and collect the display names of the room heroes from a + /// [`RoomSummary`]. + /// + /// Returns the display names as a list of strings. + async fn extract_heroes(&self, heroes: &[RoomHero]) -> StoreResult> { + let own_user_id = self.own_user_id().as_str(); + + let mut names = Vec::with_capacity(heroes.len()); + let heroes = heroes.iter().filter(|hero| hero.user_id != own_user_id); + + for hero in heroes { + if let Some(display_name) = &hero.display_name { + names.push(display_name.clone()); + } else { + match self.get_member(&hero.user_id).await { + Ok(Some(member)) => { + names.push(member.name().to_owned()); + } + Ok(None) => { + warn!("Ignoring hero, no member info for {}", hero.user_id); + } + Err(error) => { + warn!("Ignoring hero, error getting member: {}", error); + } + } + } + } + + Ok(names) } /// Compute the room summary with the data present in the store.