rooms: Store heroes as a complete user profile.

Clippy

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

@@ -17,6 +17,7 @@ use std::sync::RwLock as SyncRwLock;
 use std::{
     collections::{BTreeMap, HashSet},
     mem,
+    ops::Deref,
     sync::{atomic::AtomicBool, Arc},
 };
 
@@ -110,26 +111,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
     }
 }
 
@@ -627,25 +638,32 @@ impl Room {
 
         let own_user_id = self.own_user_id().as_str();
 
-        let (heroes, guessed_num_members): (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 {
-                    continue;
-                }
-                if let Some(member) = self.get_member(&user_id).await? {
-                    names.push(member.name().to_owned());
+        let (heroes, guessed_num_members): (Vec<String>, _) = if !summary.room_heroes.is_empty() {
+            // TODO: Fallback to the local part of the user_id if the display name is
+            // missing?
+            let hero_names: Vec<String> =
+                summary.room_heroes.iter().flat_map(|h| h.display_name.clone()).collect();
+            if !hero_names.is_empty() {
+                // Straightforward path: pass through the heroes names, don't give a guess of
+                // the number of members.
+                (hero_names, None)
+            } else {
+                // Lookup the heroes, if available.
+                let heroes: Vec<&UserId> =
+                    summary.room_heroes.iter().map(|h| h.user_id.deref()).collect();
+
+                let mut names = Vec::with_capacity(heroes.len());
+                for user_id in heroes {
+                    if user_id == own_user_id {
+                        continue;
+                    }
+                    if let Some(member) = self.get_member(user_id).await? {
+                        names.push(member.name().to_owned());
+                    }
                 }
-            }
 
-            (names, None)
+                (names, None)
+            }
         } else {
             let mut members = self.members(RoomMemberships::ACTIVE).await?;
 
@@ -1099,11 +1117,13 @@ impl RoomInfo {
         if !summary.is_empty() {
             if !summary.heroes.is_empty() {
                 // Parse the user IDs from Ruma.
-                self.summary.heroes_user_ids = summary
-                    .heroes
+                self.summary.room_heroes = summary
+                    .heroes // TODO: Load the profiles?
                     .iter()
                     .filter_map(|hero| match UserId::parse(hero.as_str()) {
-                        Ok(user_id) => Some(user_id),
+                        Ok(user_id) => {
+                            Some(RoomHero { user_id, display_name: None, avatar_url: None })
+                        }
                         Err(err) => {
                             warn!("error when parsing user id from hero '{hero}': {err}");
                             None
@@ -1140,11 +1160,10 @@ 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 number of active members (invited + joined) in the room.
@@ -1451,7 +1470,7 @@ mod tests {
 
     #[cfg(feature = "experimental-sliding-sync")]
     use super::SyncInfo;
-    use super::{Room, RoomInfo, RoomState};
+    use super::{Room, RoomHero, RoomInfo, RoomState};
     #[cfg(any(feature = "experimental-sliding-sync", feature = "e2e-encryption"))]
     use crate::latest_event::LatestEvent;
     use crate::{
@@ -1478,8 +1497,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,
             },
@@ -1503,7 +1525,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,
             },
@@ -1555,7 +1581,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",
@@ -1564,8 +1590,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,
             },
@@ -1595,8 +1624,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,
@@ -704,10 +704,17 @@ 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().filter_map(|hero| hero.user_id.as_ref()).cloned().collect(),
-            heroes.iter().filter_map(|hero| hero.name.as_ref()).cloned().collect(),
+            heroes
+                .iter()
+                .filter_map(|hero| {
+                    Some(RoomHero {
+                        user_id: hero.user_id.clone()?, // Filter out heroes without a user id.
+                        display_name: hero.name.clone(),
+                        avatar_url: hero.avatar.clone(),
+                    })
+                })
+                .collect(),
         );
     }
 
@@ -742,15 +749,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]
@@ -1296,11 +1304,12 @@ mod tests {
         room.heroes = Some(vec![
             assign!(v4::SlidingSyncRoomHero::default(), {
                 user_id: Some(gordon),
-                name: Some("Gordon".to_string()),
+                name: Some("Gordon".to_owned()),
             }),
             assign!(v4::SlidingSyncRoomHero::default(), {
                 user_id: Some(alice),
-                name: Some("Alice".to_string()),
+                name: Some("Alice".to_owned()),
+                avatar: Some(owned_mxc_uri!("mxc://e.uk/med1")),
             }),
         ]);
         let response = response_with_room(room_id, room).await;
@@ -1315,7 +1324,18 @@ mod tests {
         // And heroes are part of the summary.
         assert_eq!(
             client_room.clone_info().summary.heroes(),
-            &["@gordon:e.uk".to_string(), "@alice:e.uk".to_string()]
+            &[
+                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"))
+                },
+            ]
         );
     }