ffi: Expose room heroes (adding support for avatars). (#3503)

This PR makes 2 changes:
- Updates the storage of room heroes to be a single array containing the user's complete profile.
- Exposes these to the FFI so that client apps can use these for avatar colours/clusters.

Closes #2702 (again, now with avatars 🖼️)

---

* rooms: Store heroes as a complete user profile.

* ffi: Expose room heroes on Room and RoomInfo.

* chore: Remove TODO comment.

* Update crates/matrix-sdk-base/src/rooms/normal.rs

Signed-off-by: Benjamin Bouvier <[email protected]>

---------

Signed-off-by: Benjamin Bouvier <[email protected]>
Co-authored-by: Benjamin Bouvier <[email protected]>

Author: pixlwave

bindings/matrix-sdk-ffi/src/room.rs

@@ -4,7 +4,7 @@ use anyhow::{Context, Result};
 use matrix_sdk::{
     event_cache::paginator::PaginatorError,
     room::{power_levels::RoomPowerLevelChanges, Room as SdkRoom, RoomMemberRole},
-    ComposerDraft, RoomMemberships, RoomState,
+    ComposerDraft, RoomHero as SdkRoomHero, RoomMemberships, RoomState,
 };
 use matrix_sdk_ui::timeline::{PaginationError, RoomExt, TimelineFocus};
 use mime::Mime;
@@ -126,6 +126,11 @@ impl Room {
         self.inner.state().into()
     }
 
+    /// Returns the room heroes for this room.
+    pub fn heroes(&self) -> Vec<RoomHero> {
+        self.inner.heroes().into_iter().map(Into::into).collect()
+    }
+
     /// Is there a non expired membership with application "m.call" and scope
     /// "m.room" in this room.
     pub fn has_active_room_call(&self) -> bool {
@@ -776,6 +781,27 @@ impl RoomMembersIterator {
     }
 }
 
+/// Information about a member considered to be a room hero.
+#[derive(uniffi::Record)]
+pub struct RoomHero {
+    /// The user ID of the hero.
+    user_id: String,
+    /// The display name of the hero.
+    display_name: Option<String>,
+    /// The avatar URL of the hero.
+    avatar_url: Option<String>,
+}
+
+impl From<SdkRoomHero> for RoomHero {
+    fn from(value: SdkRoomHero) -> Self {
+        Self {
+            user_id: value.user_id.to_string(),
+            display_name: value.display_name.clone(),
+            avatar_url: value.avatar_url.as_ref().map(ToString::to_string),
+        }
+    }
+}
+
 /// An update for a particular user's power level within the room.
 #[derive(uniffi::Record)]
 pub struct UserPowerLevelUpdate {

bindings/matrix-sdk-ffi/src/room_info.rs

@@ -3,7 +3,9 @@ use std::collections::HashMap;
 use matrix_sdk::RoomState;
 
 use crate::{
-    notification_settings::RoomNotificationMode, room::Membership, room_member::RoomMember,
+    notification_settings::RoomNotificationMode,
+    room::{Membership, RoomHero},
+    room_member::RoomMember,
 };
 
 #[derive(uniffi::Record)]
@@ -30,6 +32,7 @@ pub struct RoomInfo {
     /// Can be missing if the room membership invite event is missing from the
     /// store.
     inviter: Option<RoomMember>,
+    heroes: Vec<RoomHero>,
     active_members_count: u64,
     invited_members_count: u64,
     joined_members_count: u64,
@@ -85,6 +88,7 @@ impl RoomInfo {
                     .map(Into::into),
                 _ => None,
             },
+            heroes: room.heroes().into_iter().map(Into::into).collect(),
             active_members_count: room.active_members_count(),
             invited_members_count: room.invited_members_count(),
             joined_members_count: room.joined_members_count(),

crates/matrix-sdk-base/src/lib.rs

@@ -52,8 +52,8 @@ pub use http;
 pub use matrix_sdk_crypto as crypto;
 pub use once_cell;
 pub use rooms::{
-    DisplayName, Room, RoomCreateWithCreatorEventContent, RoomInfo, RoomInfoUpdate, RoomMember,
-    RoomMemberships, RoomState, RoomStateFilter,
+    DisplayName, Room, RoomCreateWithCreatorEventContent, RoomHero, RoomInfo, RoomInfoUpdate,
+    RoomMember, RoomMemberships, RoomState, RoomStateFilter,
 };
 pub use store::{
     ComposerDraft, StateChanges, StateStore, StateStoreDataKey, StateStoreDataValue, StoreError,

crates/matrix-sdk-base/src/rooms/mod.rs

@@ -11,7 +11,7 @@ use std::{
 
 use bitflags::bitflags;
 pub use members::RoomMember;
-pub use normal::{Room, RoomInfo, RoomInfoUpdate, RoomState, RoomStateFilter};
+pub use normal::{Room, RoomHero, RoomInfo, RoomInfoUpdate, RoomState, RoomStateFilter};
 use ruma::{
     assign,
     events::{

crates/matrix-sdk-base/src/rooms/normal.rs

@@ -110,26 +110,36 @@ pub struct Room {
 /// calculate the room display name.
 #[derive(Clone, Debug, Default, Serialize, Deserialize)]
 pub struct RoomSummary {
-    /// The heroes of the room, members that should be used for the room display
-    /// name.
+    /// The heroes of the room, members that can be used as a fallback for the
+    /// room's display name or avatar if these haven't been set.
     ///
     /// This was called `heroes` and contained raw `String`s of the `UserId`
-    /// before; changing the field's name helped with avoiding a migration.
+    /// before. Following this it was called `heroes_user_ids` and a
+    /// complimentary `heroes_names` existed too; changing the field's name
+    /// helped with avoiding a migration.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub(crate) heroes_user_ids: Vec<OwnedUserId>,
-    /// The heroes names, as returned by a server, if available.
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub(crate) heroes_names: Vec<String>,
+    pub(crate) room_heroes: Vec<RoomHero>,
     /// The number of members that are considered to be joined to the room.
     pub(crate) joined_member_count: u64,
     /// The number of members that are considered to be invited to the room.
     pub(crate) invited_member_count: u64,
 }
 
+/// Information about a member considered to be a room hero.
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub struct RoomHero {
+    /// The user id of the hero.
+    pub user_id: OwnedUserId,
+    /// The display name of the hero.
+    pub display_name: Option<String>,
+    /// The avatar url of the hero.
+    pub avatar_url: Option<OwnedMxcUri>,
+}
+
 #[cfg(test)]
 impl RoomSummary {
-    pub(crate) fn heroes(&self) -> &[OwnedUserId] {
-        &self.heroes_user_ids
+    pub(crate) fn heroes(&self) -> &[RoomHero] {
+        &self.room_heroes
     }
 }
 
@@ -517,21 +527,26 @@ impl Room {
         // 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_guess): (Vec<String>, _) = if !summary.heroes_names.is_empty() {
-            // Straightforward path: pass through the heroes names, don't give a guess of
-            // the number of members.
-            (summary.heroes_names, None)
-        } else if !summary.heroes_user_ids.is_empty() {
-            // Use the heroes, if available.
-            let heroes = summary.heroes_user_ids;
-
-            let mut names = Vec::with_capacity(heroes.len());
-            for user_id in heroes {
-                if user_id == own_user_id {
+        let (heroes, num_joined_guess): (Vec<String>, _) = 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;
                 }
-                if let Some(member) = self.get_member(&user_id).await? {
-                    names.push(member.name().to_owned());
+                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);
+                    }
                 }
             }
 
@@ -694,6 +709,11 @@ impl Room {
         Ok(members)
     }
 
+    /// Get the heroes for this room.
+    pub fn heroes(&self) -> Vec<RoomHero> {
+        self.inner.read().heroes().to_vec()
+    }
+
     /// Get the list of `RoomMember`s that are considered to be joined members
     /// of this room.
     #[deprecated = "Use members with RoomMemberships::JOIN instead"]
@@ -1128,7 +1148,16 @@ impl RoomInfo {
 
         if !summary.is_empty() {
             if !summary.heroes.is_empty() {
-                self.summary.heroes_user_ids = summary.heroes.clone();
+                self.summary.room_heroes = summary
+                    .heroes
+                    .iter()
+                    .map(|hero_id| RoomHero {
+                        user_id: hero_id.to_owned(),
+                        display_name: None,
+                        avatar_url: None,
+                    })
+                    .collect();
+
                 changed = true;
             }
 
@@ -1158,11 +1187,15 @@ impl RoomInfo {
         self.summary.invited_member_count = count;
     }
 
-    /// Updates the heroes user ids.
+    /// Updates the room heroes.
     #[cfg(feature = "experimental-sliding-sync")]
-    pub(crate) fn update_heroes(&mut self, heroes: Vec<OwnedUserId>, names: Vec<String>) {
-        self.summary.heroes_user_ids = heroes;
-        self.summary.heroes_names = names;
+    pub(crate) fn update_heroes(&mut self, heroes: Vec<RoomHero>) {
+        self.summary.room_heroes = heroes;
+    }
+
+    /// The heroes for this room.
+    pub fn heroes(&self) -> &[RoomHero] {
+        &self.summary.room_heroes
     }
 
     /// The number of active members (invited + joined) in the room.
@@ -1509,7 +1542,7 @@ mod tests {
 
     #[cfg(feature = "experimental-sliding-sync")]
     use super::SyncInfo;
-    use super::{compute_display_name_from_heroes, Room, RoomInfo, RoomState};
+    use super::{compute_display_name_from_heroes, Room, RoomHero, RoomInfo, RoomState};
     #[cfg(any(feature = "experimental-sliding-sync", feature = "e2e-encryption"))]
     use crate::latest_event::LatestEvent;
     use crate::{
@@ -1536,8 +1569,11 @@ mod tests {
                 notification_count: 2,
             },
             summary: RoomSummary {
-                heroes_user_ids: vec![owned_user_id!("@somebody:example.org")],
-                heroes_names: vec![],
+                room_heroes: vec![RoomHero {
+                    user_id: owned_user_id!("@somebody:example.org"),
+                    display_name: None,
+                    avatar_url: None,
+                }],
                 joined_member_count: 5,
                 invited_member_count: 0,
             },
@@ -1562,7 +1598,11 @@ mod tests {
                 "notification_count": 2,
             },
             "summary": {
-                "heroes_user_ids": ["@somebody:example.org"],
+                "room_heroes": [{
+                    "user_id": "@somebody:example.org",
+                    "display_name": null,
+                    "avatar_url": null
+                }],
                 "joined_member_count": 5,
                 "invited_member_count": 0,
             },
@@ -1614,7 +1654,7 @@ mod tests {
         // The following JSON should never change if we want to be able to read in old
         // cached state
 
-        use ruma::owned_user_id;
+        use ruma::{owned_mxc_uri, owned_user_id};
         let info_json = json!({
             "room_id": "!gda78o:server.tld",
             "room_state": "Invited",
@@ -1623,8 +1663,11 @@ mod tests {
                 "notification_count": 2,
             },
             "summary": {
-                "heroes_user_ids": ["@somebody:example.org"],
-                "heroes_names": ["Somebody"],
+                "room_heroes": [{
+                    "user_id": "@somebody:example.org",
+                    "display_name": "Somebody",
+                    "avatar_url": "mxc://example.org/abc"
+                }],
                 "joined_member_count": 5,
                 "invited_member_count": 0,
             },
@@ -1655,8 +1698,14 @@ mod tests {
         assert_eq!(info.room_state, RoomState::Invited);
         assert_eq!(info.notification_counts.highlight_count, 1);
         assert_eq!(info.notification_counts.notification_count, 2);
-        assert_eq!(info.summary.heroes_user_ids, vec![owned_user_id!("@somebody:example.org")]);
-        assert_eq!(info.summary.heroes_names, vec!["Somebody".to_owned()]);
+        assert_eq!(
+            info.summary.room_heroes,
+            vec![RoomHero {
+                user_id: owned_user_id!("@somebody:example.org"),
+                display_name: Some("Somebody".to_owned()),
+                avatar_url: Some(owned_mxc_uri!("mxc://example.org/abc")),
+            }]
+        );
         assert_eq!(info.summary.joined_member_count, 5);
         assert_eq!(info.summary.invited_member_count, 0);
         assert!(info.members_synced);

crates/matrix-sdk-base/src/sliding_sync.rs

@@ -38,7 +38,7 @@ use crate::RoomMemberships;
 use crate::{
     error::Result,
     read_receipts::{compute_unread_counts, PreviousEventsProvider},
-    rooms::RoomState,
+    rooms::{normal::RoomHero, RoomState},
     store::{ambiguity_map::AmbiguityCache, StateChanges, Store},
     sync::{JoinedRoomUpdate, LeftRoomUpdate, Notification, RoomUpdates, SyncResponse},
     Room, RoomInfo,
@@ -711,10 +711,15 @@ fn process_room_properties(room_data: &v4::SlidingSyncRoom, room_info: &mut Room
     }
 
     if let Some(heroes) = &room_data.heroes {
-        // Filter out all the heroes which don't have a user id or name.
         room_info.update_heroes(
-            heroes.iter().map(|hero| &hero.user_id).cloned().collect(),
-            heroes.iter().filter_map(|hero| hero.name.as_ref()).cloned().collect(),
+            heroes
+                .iter()
+                .map(|hero| RoomHero {
+                    user_id: hero.user_id.clone(),
+                    display_name: hero.name.clone(),
+                    avatar_url: hero.avatar.clone(),
+                })
+                .collect(),
         );
     }
 
@@ -750,15 +755,16 @@ mod tests {
             AnySyncMessageLikeEvent, AnySyncTimelineEvent, GlobalAccountDataEventContent,
             StateEventContent,
         },
-        mxc_uri, room_alias_id, room_id,
+        mxc_uri, owned_mxc_uri, owned_user_id, room_alias_id, room_id,
         serde::Raw,
         uint, user_id, JsOption, MxcUri, OwnedRoomId, OwnedUserId, RoomAliasId, RoomId, UserId,
     };
     use serde_json::json;
 
     use super::cache_latest_events;
     use crate::{
-        store::MemoryStore, test_utils::logged_in_base_client, BaseClient, Room, RoomState,
+        rooms::normal::RoomHero, store::MemoryStore, test_utils::logged_in_base_client, BaseClient,
+        Room, RoomState,
     };
 
     #[async_test]
@@ -1356,6 +1362,7 @@ mod tests {
             }),
             assign!(v4::SlidingSyncRoomHero::new(alice), {
                 name: Some("Alice".to_owned()),
+                avatar: Some(owned_mxc_uri!("mxc://e.uk/med1"))
             }),
         ]);
         let response = response_with_room(room_id, room);
@@ -1370,7 +1377,18 @@ mod tests {
         // And heroes are part of the summary.
         assert_eq!(
             client_room.clone_info().summary.heroes(),
-            &["@gordon:e.uk".to_owned(), "@alice:e.uk".to_owned()]
+            &[
+                RoomHero {
+                    user_id: owned_user_id!("@gordon:e.uk"),
+                    display_name: Some("Gordon".to_owned()),
+                    avatar_url: None
+                },
+                RoomHero {
+                    user_id: owned_user_id!("@alice:e.uk"),
+                    display_name: Some("Alice".to_owned()),
+                    avatar_url: Some(owned_mxc_uri!("mxc://e.uk/med1"))
+                },
+            ]
         );
     }
 

crates/matrix-sdk/src/lib.rs

@@ -23,7 +23,7 @@ pub use matrix_sdk_base::crypto;
 pub use matrix_sdk_base::{
     deserialized_responses,
     store::{ComposerDraft, DynStateStore, MemoryStore, StateStoreExt},
-    DisplayName, Room as BaseRoom, RoomCreateWithCreatorEventContent, RoomInfo,
+    DisplayName, Room as BaseRoom, RoomCreateWithCreatorEventContent, RoomHero, RoomInfo,
     RoomMember as BaseRoomMember, RoomMemberships, RoomState, SessionMeta, StateChanges,
     StateStore, StoreError,
 };