Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MSC4717: service members 🫡 #4335

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ default-members = ["benchmarks", "crates/*", "labs/*"]
resolver = "2"

[workspace.package]
rust-version = "1.80"
rust-version = "1.82"

[workspace.dependencies]
anyhow = "1.0.93"
Expand Down Expand Up @@ -56,7 +56,7 @@ proptest = { version = "1.5.0", default-features = false, features = ["std"] }
rand = "0.8.5"
reqwest = { version = "0.12.4", default-features = false }
rmp-serde = "1.3.0"
ruma = { git = "https://github.com/ruma/ruma.git", rev = "35fda7f2f7811156df2e60b223dbf136fc143bc8", features = [
ruma = { git = "https://github.com/ruma/ruma", rev = "c91499fc464adc865a7c99d0ce0b35982ad96711", features = [
"client-api-c",
"compat-upload-signatures",
"compat-user-id",
Expand All @@ -69,8 +69,9 @@ ruma = { git = "https://github.com/ruma/ruma.git", rev = "35fda7f2f7811156df2e60
"unstable-msc3489",
"unstable-msc4075",
"unstable-msc4140",
"unstable-msc4171",
] }
ruma-common = { git = "https://github.com/ruma/ruma.git", rev = "35fda7f2f7811156df2e60b223dbf136fc143bc8" }
ruma-common = { git = "https://github.com/ruma/ruma", rev = "c91499fc464adc865a7c99d0ce0b35982ad96711" }
serde = "1.0.151"
serde_html_form = "0.2.0"
serde_json = "1.0.91"
Expand Down
8 changes: 8 additions & 0 deletions crates/matrix-sdk-base/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ All notable changes to this project will be documented in this file.

## [Unreleased] - ReleaseDate

### Features

- Introduced support for
[MSC4171](https://github.com/matrix-org/matrix-rust-sdk/pull/4335), enabling
the designation of certain users as service members. These flagged users are
excluded from the room display name calculation.
([#4335](https://github.com/matrix-org/matrix-rust-sdk/pull/4335))

### Bug Fixes

- Fix an off-by-one error in the `ObservableMap` when the `remove()` method is
Expand Down
170 changes: 160 additions & 10 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
sync::{atomic::AtomicBool, Arc},
};

use as_variant::as_variant;
use bitflags::bitflags;
use eyeball::{SharedObservable, Subscriber};
use futures_util::{Stream, StreamExt};
Expand All @@ -35,6 +36,7 @@ use ruma::{
call::member::{CallMemberStateKey, MembershipData},
direct::OwnedDirectUserIdentifier,
ignored_user_list::IgnoredUserListEventContent,
member_hints::MemberHintsEventContent,
receipt::{Receipt, ReceiptThread, ReceiptType},
room::{
avatar::{self, RoomAvatarEventContent},
Expand All @@ -50,7 +52,7 @@ use ruma::{
},
tag::{TagEventContent, Tags},
AnyRoomAccountDataEvent, AnyStrippedStateEvent, AnySyncStateEvent,
RoomAccountDataEventType,
RoomAccountDataEventType, SyncStateEvent,
},
room::RoomType,
serde::Raw,
Expand All @@ -59,7 +61,7 @@ use ruma::{
};
use serde::{Deserialize, Serialize};
use tokio::sync::broadcast;
use tracing::{debug, field::debug, info, instrument, warn};
use tracing::{debug, field::debug, info, instrument, trace, warn};

use super::{
members::MemberRoomInfo, BaseRoomInfo, RoomCreateWithCreatorEventContent, RoomDisplayName,
Expand All @@ -68,7 +70,9 @@ use super::{
#[cfg(feature = "experimental-sliding-sync")]
use crate::latest_event::LatestEvent;
use crate::{
deserialized_responses::{DisplayName, MemberEvent, RawSyncOrStrippedState},
deserialized_responses::{
DisplayName, MemberEvent, RawSyncOrStrippedState, SyncOrStrippedState,
},
notification_settings::RoomNotificationMode,
read_receipts::RoomReadReceipts,
store::{DynStateStore, Result as StoreResult, StateStoreExt},
Expand Down Expand Up @@ -222,6 +226,18 @@ impl From<&MembershipState> for RoomState {
/// try to behave similarly here.
const NUM_HEROES: usize = 5;

/// A filter to remove our own user and the users specified in the member hints
/// state event, so called service members, from the list of heroes.
///
/// The heroes will then be used to calculate a display name for the room if one
/// wasn't explicitly defined.
fn heroes_filter<'a>(
own_user_id: &'a UserId,
member_hints: &'a MemberHintsEventContent,
) -> impl Fn(&UserId) -> bool + use<'a> {
move |user_id| user_id != own_user_id && !member_hints.service_members.contains(user_id)
}
poljar marked this conversation as resolved.
Show resolved Hide resolved

impl Room {
/// The size of the latest_encrypted_events RingBuffer
// SAFETY: `new_unchecked` is safe because 10 is not zero.
Expand Down Expand Up @@ -678,12 +694,16 @@ impl Room {
///
/// Returns the display names as a list of strings.
async fn extract_heroes(&self, heroes: &[RoomHero]) -> StoreResult<Vec<String>> {
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);
let own_user_id = self.own_user_id();
let member_hints = self.get_member_hints().await?;

// 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);
let heroes_filter = |hero: &&RoomHero| heroes_filter(&hero.user_id);

for hero in heroes {
for hero in heroes.iter().filter(heroes_filter) {
if let Some(display_name) = &hero.display_name {
names.push(display_name.clone());
} else {
Expand All @@ -710,12 +730,30 @@ impl Room {
///
/// Returns a `(heroes_names, num_joined_invited)` tuple.
async fn compute_summary(&self) -> StoreResult<(Vec<String>, u64)> {
let member_hints = self.get_member_hints().await?;

// Construct a filter that is specific to this own user id, set of member hints,
// and accepts a `RoomMember` type.
let heroes_filter = heroes_filter(&self.own_user_id, &member_hints);
let heroes_filter = |u: &RoomMember| heroes_filter(u.user_id());

let mut members = self.members(RoomMemberships::JOIN | RoomMemberships::INVITE).await?;

// If we have some service members, they shouldn't count to the number of
// joined/invited members, 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 = members
.iter()
.filter(|member| member_hints.service_members.contains(member.user_id()))
.count();

// We can make a good prediction of the total number of joined and invited
// members here. This might be incorrect if the database info is
// outdated.
let num_joined_invited = members.len() as u64;
//
// Note: Subtracting here is fine because `num_service_members` is a subset of
// `members.len()` due to the above filter operation.
let num_joined_invited = members.len() - num_service_members;

if num_joined_invited == 0
|| (num_joined_invited == 1 && members[0].user_id() == self.own_user_id)
Expand All @@ -729,12 +767,34 @@ impl Room {

let heroes = members
.into_iter()
.filter(|u| u.user_id() != self.own_user_id)
.filter(heroes_filter)
.take(NUM_HEROES)
.map(|u| u.name().to_owned())
.collect();

Ok((heroes, num_joined_invited))
trace!(
?heroes,
num_joined_invited,
num_service_members,
"Computed a room summary since we didn't receive one."
);

Ok((heroes, num_joined_invited as u64))
}

async fn get_member_hints(&self) -> StoreResult<MemberHintsEventContent> {
Ok(self
.store
.get_state_event_static::<MemberHintsEventContent>(self.room_id())
.await?
.and_then(|event| {
event
.deserialize()
.inspect_err(|e| warn!("Couldn't deserialize the member hints event: {e}"))
.ok()
})
.and_then(|event| as_variant!(event, SyncOrStrippedState::Sync(SyncStateEvent::Original(e)) => e.content))
.unwrap_or_default())
}

/// Returns the cached computed display name, if available.
Expand Down Expand Up @@ -1854,6 +1914,7 @@ fn compute_display_name_from_heroes(
#[cfg(test)]
mod tests {
use std::{
collections::BTreeSet,
ops::{Not, Sub},
str::FromStr,
sync::Arc,
Expand Down Expand Up @@ -1894,6 +1955,7 @@ mod tests {
OwnedEventId, OwnedUserId, UserId,
};
use serde_json::json;
use similar_asserts::assert_eq;
use stream_assert::{assert_pending, assert_ready};

use super::{compute_display_name_from_heroes, Room, RoomHero, RoomInfo, RoomState, SyncInfo};
Expand Down Expand Up @@ -2546,6 +2608,53 @@ mod tests {
);
}

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

let matthew = user_id!("@sahasrhala:example.org");
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(), matthew.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(matthew.into(), f.member(matthew).display_name("Matthew").into_raw());
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::Calculated("Matthew".to_owned())
);
}

#[async_test]
async fn test_display_name_dm_joined_no_heroes() {
let (store, room) = make_room_test_helper(RoomState::Joined);
Expand Down Expand Up @@ -2573,6 +2682,47 @@ mod tests {
);
}

#[async_test]
async fn test_display_name_dm_joined_no_heroes_service_members() {
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");
let bot = user_id!("@bot:example.org");

let mut changes = StateChanges::new("".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(matthew.into(), f.member(matthew).display_name("Matthew").into_raw());
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();

assert_eq!(
room.compute_display_name().await.unwrap(),
RoomDisplayName::Calculated("Matthew".to_owned())
);
}

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