sdk-base: store heroes as OwnedUserIds in our own RoomSummary

This is the right goal, and Ruma will be updated to reflect that the
`heroes` field should contain `OwnedUserId` too in [1].

This simplifies the code a bit, and avoids a round-trip encoding
user-ids into a string then decoding them from a string later, in the
case of sliding sync and room name computation.

[1] https://github.com/ruma/ruma/pull/1822
This commit is contained in:
Benjamin Bouvier
2024-05-23 19:33:01 +02:00
parent 2851a42fed
commit 56be9d5461
3 changed files with 99 additions and 67 deletions

View File

@@ -835,7 +835,7 @@ impl BaseClient {
let mut room_info = room.clone_info();
room_info.mark_as_joined();
room_info.update_summary(&new_info.summary);
room_info.update_from_ruma_summary(&new_info.summary);
room_info.set_prev_batch(new_info.timeline.prev_batch.as_deref());
room_info.mark_state_fully_synced();

View File

@@ -113,17 +113,21 @@ pub struct Room {
pub struct RoomSummary {
/// The heroes of the room, members that should be used for the room display
/// name.
heroes: Vec<String>,
///
/// This was called `heroes` and contained raw `String`s of the `UserId`
/// before; changing the field's name helped with avoiding a migration.
#[serde(default)]
pub(crate) heroes_user_ids: Vec<OwnedUserId>,
/// The number of members that are considered to be joined to the room.
joined_member_count: u64,
pub(crate) joined_member_count: u64,
/// The number of members that are considered to be invited to the room.
invited_member_count: u64,
pub(crate) invited_member_count: u64,
}
#[cfg(test)]
impl RoomSummary {
pub(crate) fn heroes(&self) -> &[String] {
&self.heroes
pub(crate) fn heroes(&self) -> &[OwnedUserId] {
&self.heroes_user_ids
}
}
@@ -621,45 +625,46 @@ impl Room {
let own_user_id = self.own_user_id().as_str();
let (heroes, guessed_num_members): (Vec<RoomMember>, _) = if summary.heroes.is_empty() {
let mut members = self.members(RoomMemberships::ACTIVE).await?;
let (heroes, guessed_num_members): (Vec<RoomMember>, _) =
if summary.heroes_user_ids.is_empty() {
let mut members = self.members(RoomMemberships::ACTIVE).await?;
// Make the ordering deterministic.
members.sort_unstable_by(|lhs, rhs| lhs.name().cmp(rhs.name()));
// Make the ordering deterministic.
members.sort_unstable_by(|lhs, rhs| lhs.name().cmp(rhs.name()));
// We can make a good prediction of the total number of members here. This might
// be incorrect if the database info is outdated.
let guessed_num_members = Some(members.len());
// We can make a good prediction of the total number of members here. This might
// be incorrect if the database info is outdated.
let guessed_num_members = Some(members.len());
(
members
.into_iter()
.take(NUM_HEROES)
.filter(|u| u.user_id() != own_user_id)
.collect(),
guessed_num_members,
)
} else {
let mut heroes = summary.heroes;
(
members
.into_iter()
.take(NUM_HEROES)
.filter(|u| u.user_id() != own_user_id)
.collect(),
guessed_num_members,
)
} else {
let mut heroes = summary.heroes_user_ids;
// Make the ordering deterministic.
heroes.sort_unstable();
// Make the ordering deterministic.
heroes.sort_unstable();
let members: Vec<_> = stream::iter(heroes.iter())
.filter_map(|u| async move {
let user_id = UserId::parse(u.as_str()).ok()?;
if user_id == own_user_id {
return None;
}
self.get_member(&user_id).await.transpose()
})
.collect()
.await;
let members: Vec<_> = stream::iter(heroes.iter())
.filter_map(|user_id| async move {
// Filter out the current user.
if user_id == own_user_id {
return None;
}
self.get_member(&user_id).await.transpose()
})
.collect()
.await;
let members: StoreResult<Vec<_>> = members.into_iter().collect();
let members: StoreResult<Vec<_>> = members.into_iter().collect();
(members?, None)
};
(members?, None)
};
let (num_joined, num_invited) = match self.state() {
RoomState::Invited => {
@@ -1081,15 +1086,27 @@ impl RoomInfo {
self.notification_counts = notification_counts;
}
/// Update the RoomSummary.
/// Update the RoomSummary from a Ruma `RoomSummary`.
///
/// Returns true if the Summary modified the info, false otherwise.
pub fn update_summary(&mut self, summary: &RumaSummary) -> bool {
/// Returns true if any field has been updated, false otherwise.
pub fn update_from_ruma_summary(&mut self, summary: &RumaSummary) -> bool {
let mut changed = false;
if !summary.is_empty() {
if !summary.heroes.is_empty() {
self.summary.heroes = summary.heroes.clone();
// Parse the user IDs from Ruma.
self.summary.heroes_user_ids = summary
.heroes
.iter()
.filter_map(|hero| match UserId::parse(hero.as_str()) {
Ok(user_id) => Some(user_id),
Err(err) => {
warn!("error when parsing user id from hero '{hero}': {err}");
None
}
})
.collect();
changed = true;
}
@@ -1107,6 +1124,21 @@ impl RoomInfo {
changed
}
/// Updates the joined member count.
pub(crate) fn update_joined_member_count(&mut self, count: u64) {
self.summary.joined_member_count = count;
}
/// Updates the invited member count.
pub(crate) fn update_invited_member_count(&mut self, count: u64) {
self.summary.invited_member_count = count;
}
/// Updates the heroes user ids.
pub(crate) fn update_heroes(&mut self, heroes: Vec<OwnedUserId>) {
self.summary.heroes_user_ids = heroes;
}
/// The number of active members (invited + joined) in the room.
///
/// The return value is saturated at `u64::MAX`.
@@ -1425,6 +1457,8 @@ mod tests {
// This test exists to make sure we don't accidentally change the
// serialized format for `RoomInfo`.
use ruma::owned_user_id;
use super::RoomSummary;
use crate::{rooms::BaseRoomInfo, sync::UnreadNotificationsCount};
@@ -1436,7 +1470,7 @@ mod tests {
notification_count: 2,
},
summary: RoomSummary {
heroes: vec!["Somebody".to_owned()],
heroes_user_ids: vec![owned_user_id!("@somebody:example.org")],
joined_member_count: 5,
invited_member_count: 0,
},
@@ -1460,7 +1494,7 @@ mod tests {
"notification_count": 2,
},
"summary": {
"heroes": ["Somebody"],
"heroes_user_ids": ["@somebody:example.org"],
"joined_member_count": 5,
"invited_member_count": 0,
},
@@ -1511,6 +1545,8 @@ 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;
let info_json = json!({
"room_id": "!gda78o:server.tld",
"room_state": "Invited",
@@ -1519,7 +1555,7 @@ mod tests {
"notification_count": 2,
},
"summary": {
"heroes": ["Somebody"],
"heroes_user_ids": ["@somebody:example.org"],
"joined_member_count": 5,
"invited_member_count": 0,
},
@@ -1549,7 +1585,7 @@ 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, vec!["Somebody".to_owned()]);
assert_eq!(info.summary.heroes_user_ids, vec![owned_user_id!("@somebody:example.org")]);
assert_eq!(info.summary.joined_member_count, 5);
assert_eq!(info.summary.invited_member_count, 0);
assert!(info.members_synced);
@@ -1864,7 +1900,7 @@ mod tests {
changes.add_stripped_member(room_id, me, make_stripped_member_event(me, "Me"));
store.save_changes(&changes).await.unwrap();
room.inner.update_if(|info| info.update_summary(&summary));
room.inner.update_if(|info| info.update_from_ruma_summary(&summary));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
@@ -1916,7 +1952,7 @@ mod tests {
store.save_changes(&changes).await.unwrap();
room.inner.update_if(|info| info.update_summary(&summary));
room.inner.update_if(|info| info.update_from_ruma_summary(&summary));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::Calculated("Matthew".to_owned())
@@ -1995,7 +2031,7 @@ mod tests {
joined_member_count: Some(7u32.into()),
heroes: vec![denis.to_string(), carol.to_string(), bob.to_string(), erica.to_string()],
});
room.inner.update_if(|info| info.update_summary(&summary));
room.inner.update_if(|info| info.update_from_ruma_summary(&summary));
assert_eq!(
room.computed_display_name().await.unwrap(),
@@ -2075,7 +2111,7 @@ mod tests {
store.save_changes(&changes).await.unwrap();
room.inner.update_if(|info| info.update_summary(&summary));
room.inner.update_if(|info| info.update_from_ruma_summary(&summary));
assert_eq!(
room.computed_display_name().await.unwrap(),
DisplayName::EmptyWas("Matthew".to_owned())

View File

@@ -21,7 +21,7 @@ use matrix_sdk_common::deserialized_responses::SyncTimelineEvent;
use ruma::events::AnyToDeviceEvent;
use ruma::{
api::client::sync::sync_events::{
v3::{self, InvitedRoom, RoomSummary},
v3::{self, InvitedRoom},
v4,
},
events::{AnyRoomAccountDataEvent, AnySyncStateEvent, AnySyncTimelineEvent},
@@ -38,7 +38,7 @@ use crate::RoomMemberships;
use crate::{
error::Result,
read_receipts::{compute_unread_counts, PreviousEventsProvider},
rooms::RoomState,
rooms::{normal::RoomSummary, RoomState},
store::{ambiguity_map::AmbiguityCache, StateChanges, Store},
sync::{JoinedRoomUpdate, LeftRoomUpdate, Notification, RoomUpdates, SyncResponse},
Room, RoomInfo,
@@ -695,23 +695,19 @@ fn process_room_properties(room_data: &v4::SlidingSyncRoom, room_info: &mut Room
// Sliding sync doesn't have a room summary, nevertheless it contains the joined
// and invited member counts, in addition to the heroes if it's been configured
// to return them (see the [`v4::RoomSubscription::include_heroes`]).
//
// Let's at least fetch the member counts, since they might be useful.
let mut room_summary = RoomSummary::new();
room_summary.invited_member_count = room_data.invited_count;
room_summary.joined_member_count = room_data.joined_count;
if let Some(heroes) = &room_data.heroes {
// It's not clear for Ruma, but the `heroes` field should be a collection of
// [`UserId`].
room_summary.heroes = heroes
.iter()
.filter_map(|hero| hero.user_id.as_ref().map(ToString::to_string))
.collect();
if let Some(count) = room_data.joined_count {
room_info.update_joined_member_count(count.into());
}
if let Some(count) = room_data.invited_count {
room_info.update_invited_member_count(count.into());
}
room_info.update_summary(&room_summary);
if let Some(heroes) = &room_data.heroes {
// Filter out all the heroes which don't have a user id.
room_info.update_heroes(
heroes.iter().filter_map(|hero| hero.user_id.as_ref()).cloned().collect(),
);
}
room_info.set_prev_batch(room_data.prev_batch.as_deref());