Skip to content

Commit

Permalink
fix(base): Subtract the number of service members from the number joi…
Browse files Browse the repository at this point in the history
…ned members

This patch fixes an edge case where the member is alone in the room with
a service member. We already subtracted the number of service members
in the case we calculated the room summary ourselves, but we did not do
so when the server provided the room summary.

This lead to the room, instead of being called `Empty`, being called
`Foo and N others`.
  • Loading branch information
poljar committed Dec 12, 2024
1 parent 9b77b13 commit 6100f26
Showing 1 changed file with 67 additions and 12 deletions.
79 changes: 67 additions & 12 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,16 +646,20 @@ impl Room {
&self,
summary: RoomSummary,
) -> StoreResult<RoomDisplayName> {
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)
let (heroes, num_service_members, num_joined_invited_guess) = if !summary
.room_heroes
.is_empty()
{
let (heroes, num_service_members) = self.extract_heroes(&summary.room_heroes).await?;
(heroes, num_service_members, None)
} else {
let (heroes, num_joined_invited) = self.compute_summary().await?;
(heroes, Some(num_joined_invited))
let (heroes, num_service_members, num_joined_invited) = self.compute_summary().await?;
(heroes, num_service_members, Some(num_joined_invited))
};

let summary_member_count = (summary.joined_member_count + summary.invited_member_count)
.saturating_sub(num_service_members);

let num_joined_invited = if self.state() == RoomState::Invited {
// when we were invited we don't have a proper summary, we have to do best
// guessing
Expand Down Expand Up @@ -693,11 +697,20 @@ impl Room {
/// [`RoomSummary`].
///
/// Returns the display names as a list of strings.
async fn extract_heroes(&self, heroes: &[RoomHero]) -> StoreResult<Vec<String>> {
async fn extract_heroes(&self, heroes: &[RoomHero]) -> StoreResult<(Vec<String>, u64)> {
let mut names = Vec::with_capacity(heroes.len());
let own_user_id = self.own_user_id();
let member_hints = self.get_member_hints().await?;

// If we have some service members in the heroes, that means that they are also
// part of the joined member counts. They shouldn't be so, otherwise
// we'll wrongly assume that there are more members in the room than
// they are for the "Bob and 2 others" case.
let num_service_members = heroes
.iter()
.filter(|hero| member_hints.service_members.contains(&hero.user_id))
.count() as u64;

// Construct a filter that is specific to this own user id, set of member hints,
// and accepts a `RoomHero` type.
let heroes_filter = heroes_filter(own_user_id, &member_hints);
Expand All @@ -721,15 +734,15 @@ impl Room {
}
}

Ok(names)
Ok((names, num_service_members))
}

/// Compute the room summary with the data present in the store.
///
/// The summary might be incorrect if the database info is outdated.
///
/// Returns a `(heroes_names, num_joined_invited)` tuple.
async fn compute_summary(&self) -> StoreResult<(Vec<String>, u64)> {
async fn compute_summary(&self) -> StoreResult<(Vec<String>, u64, u64)> {
let member_hints = self.get_member_hints().await?;

// Construct a filter that is specific to this own user id, set of member hints,
Expand Down Expand Up @@ -779,7 +792,7 @@ impl Room {
"Computed a room summary since we didn't receive one."
);

Ok((heroes, num_joined_invited as u64))
Ok((heroes, num_service_members as u64, num_joined_invited as u64))
}

async fn get_member_hints(&self) -> StoreResult<MemberHintsEventContent> {
Expand Down Expand Up @@ -2619,7 +2632,7 @@ mod tests {

let mut changes = StateChanges::new("".to_owned());
let summary = assign!(RumaSummary::new(), {
joined_member_count: Some(2u32.into()),
joined_member_count: Some(3u32.into()),
heroes: vec![me.to_owned(), matthew.to_owned(), bot.to_owned()],
});

Expand Down Expand Up @@ -2655,6 +2668,48 @@ mod tests {
);
}

#[async_test]
async fn test_display_name_dm_joined_alone_with_service_members() {
let (store, room) = make_room_test_helper(RoomState::Joined);
let room_id = room_id!("!test:localhost");

let me = user_id!("@me:example.org");
let bot = user_id!("@bot:example.org");

let mut changes = StateChanges::new("".to_owned());
let summary = assign!(RumaSummary::new(), {
joined_member_count: Some(2u32.into()),
heroes: vec![me.to_owned(), bot.to_owned()],
});

let f = EventFactory::new().room(room_id!("!test:localhost"));

let members = changes
.state
.entry(room_id.to_owned())
.or_default()
.entry(StateEventType::RoomMember)
.or_default();
members.insert(me.into(), f.member(me).display_name("Me").into_raw());
members.insert(bot.into(), f.member(bot).display_name("Bot").into_raw());

let member_hints_content =
f.member_hints(BTreeSet::from([bot.to_owned()])).sender(me).into_raw();
changes
.state
.entry(room_id.to_owned())
.or_default()
.entry(StateEventType::MemberHints)
.or_default()
.insert("".to_owned(), member_hints_content);

store.save_changes(&changes).await.unwrap();

room.inner.update_if(|info| info.update_from_ruma_summary(&summary));
// Bot should not contribute to the display name.
assert_eq!(room.compute_display_name().await.unwrap(), RoomDisplayName::Empty);
}

#[async_test]
async fn test_display_name_dm_joined_no_heroes() {
let (store, room) = make_room_test_helper(RoomState::Joined);
Expand Down

0 comments on commit 6100f26

Please sign in to comment.