Skip to content

Commit 7c13b60

Browse files
jmartinesppoljar
authored andcommitted
fix: Refresh timeline items when their sender's avatar URL changes
This is similar to what happens when a user display name changes, its ambiguity is calculated and if it changes, it reloads the associated timeline events. In fact, this change tries to follow the same strategy as `AmbiguityCache`.
1 parent e5a5008 commit 7c13b60

14 files changed

Lines changed: 329 additions & 25 deletions

File tree

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ use crate::{
6565
Room, RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMembersUpdate, RoomState,
6666
},
6767
store::{
68-
BaseStateStore, DynStateStore, MemoryStore, Result as StoreResult, RoomLoadSettings,
69-
StateChanges, StateStoreDataKey, StateStoreDataValue, StateStoreExt, StoreConfig,
70-
ambiguity_map::AmbiguityCache,
68+
AvatarCache, BaseStateStore, DynStateStore, MemoryStore, Result as StoreResult,
69+
RoomLoadSettings, StateChanges, StateStoreDataKey, StateStoreDataValue, StateStoreExt,
70+
StoreConfig, ambiguity_map::AmbiguityCache,
7171
},
7272
sync::{RoomUpdates, SyncResponse},
7373
};
@@ -644,6 +644,7 @@ impl BaseClient {
644644
.collect();
645645

646646
let mut ambiguity_cache = AmbiguityCache::new(self.state_store.inner.clone());
647+
let mut avatar_cache = AvatarCache::new(self.state_store.inner.clone());
647648

648649
let global_account_data_processor =
649650
processors::account_data::global(&response.account_data.events);
@@ -670,6 +671,7 @@ impl BaseClient {
670671
&room_id,
671672
requested_required_states,
672673
&mut ambiguity_cache,
674+
&mut avatar_cache,
673675
),
674676
joined_room,
675677
&mut updated_members_in_room,
@@ -693,6 +695,7 @@ impl BaseClient {
693695
&room_id,
694696
requested_required_states,
695697
&mut ambiguity_cache,
698+
&mut avatar_cache,
696699
),
697700
left_room,
698701
processors::notification::Notification::new(

crates/matrix-sdk-base/src/response_processors/room/mod.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414

1515
use ruma::RoomId;
1616

17-
use crate::{RequestedRequiredStates, store::ambiguity_map::AmbiguityCache};
17+
use crate::{
18+
RequestedRequiredStates,
19+
store::{AvatarCache, ambiguity_map::AmbiguityCache},
20+
};
1821

1922
pub mod display_name;
2023
pub mod msc4186;
@@ -25,14 +28,16 @@ pub struct RoomCreationData<'a> {
2528
room_id: &'a RoomId,
2629
requested_required_states: &'a RequestedRequiredStates,
2730
ambiguity_cache: &'a mut AmbiguityCache,
31+
avatar_cache: &'a mut AvatarCache,
2832
}
2933

3034
impl<'a> RoomCreationData<'a> {
3135
pub fn new(
3236
room_id: &'a RoomId,
3337
requested_required_states: &'a RequestedRequiredStates,
3438
ambiguity_cache: &'a mut AmbiguityCache,
39+
avatar_cache: &'a mut AvatarCache,
3540
) -> Self {
36-
Self { room_id, requested_required_states, ambiguity_cache }
41+
Self { room_id, requested_required_states, ambiguity_cache, avatar_cache }
3742
}
3843
}

crates/matrix-sdk-base/src/response_processors/room/msc4186/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ pub async fn update_any_room(
6666
) -> Result<Option<(RoomInfo, RoomUpdateKind)>> {
6767
let _timer = timer!(tracing::Level::TRACE, "update_any_room");
6868

69-
let RoomCreationData { room_id, requested_required_states, ambiguity_cache } =
69+
let RoomCreationData { room_id, requested_required_states, ambiguity_cache, avatar_cache } =
7070
room_creation_data;
7171

7272
// Read state events from the `required_state` field.
@@ -118,6 +118,7 @@ pub async fn update_any_room(
118118
raw_state_events,
119119
&mut room_info,
120120
ambiguity_cache,
121+
avatar_cache,
121122
&mut new_user_ids,
122123
state_store,
123124
#[cfg(feature = "experimental-encrypted-state-events")]
@@ -170,6 +171,7 @@ pub async fn update_any_room(
170171
room_info.update_notification_count(notification_count);
171172

172173
let ambiguity_changes = ambiguity_cache.changes.remove(room_id).unwrap_or_default();
174+
let avatar_changes = avatar_cache.remove_changes(room_id);
173175
let room_account_data = rooms_account_data.get(room_id);
174176

175177
match (room_info.state(), maybe_room_update_kind) {
@@ -188,6 +190,7 @@ pub async fn update_any_room(
188190
ephemeral,
189191
notification_count,
190192
ambiguity_changes,
193+
avatar_changes,
191194
)),
192195
)))
193196
}

crates/matrix-sdk-base/src/response_processors/room/sync_v2.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub async fn update_joined_room(
4343
notification: notification::Notification<'_>,
4444
#[cfg(feature = "e2e-encryption")] e2ee: &e2ee::E2EE<'_>,
4545
) -> Result<JoinedRoomUpdate> {
46-
let RoomCreationData { room_id, requested_required_states, ambiguity_cache } =
46+
let RoomCreationData { room_id, requested_required_states, ambiguity_cache, avatar_cache } =
4747
room_creation_data;
4848

4949
let state_store = notification.state_store;
@@ -68,6 +68,7 @@ pub async fn update_joined_room(
6868
raw_state_events,
6969
&mut room_info,
7070
ambiguity_cache,
71+
avatar_cache,
7172
&mut new_user_ids,
7273
state_store,
7374
#[cfg(feature = "experimental-encrypted-state-events")]
@@ -137,6 +138,7 @@ pub async fn update_joined_room(
137138
joined_room.ephemeral.events,
138139
notification_count,
139140
ambiguity_cache.changes.remove(room_id).unwrap_or_default(),
141+
avatar_cache.remove_changes(room_id),
140142
))
141143
}
142144

@@ -149,7 +151,7 @@ pub async fn update_left_room(
149151
notification: notification::Notification<'_>,
150152
#[cfg(feature = "e2e-encryption")] e2ee: &e2ee::E2EE<'_>,
151153
) -> Result<LeftRoomUpdate> {
152-
let RoomCreationData { room_id, requested_required_states, ambiguity_cache } =
154+
let RoomCreationData { room_id, requested_required_states, ambiguity_cache, avatar_cache } =
153155
room_creation_data;
154156

155157
#[cfg(feature = "e2e-encryption")]
@@ -172,6 +174,7 @@ pub async fn update_left_room(
172174
raw_state_events,
173175
&mut room_info,
174176
ambiguity_cache,
177+
avatar_cache,
175178
&mut (),
176179
state_store,
177180
#[cfg(feature = "experimental-encrypted-state-events")]

crates/matrix-sdk-base/src/response_processors/state_events.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ pub mod sync {
4545
use crate::response_processors::e2ee;
4646
use crate::{
4747
RoomInfo, RoomInfoNotableUpdateReasons, RoomState,
48-
store::{BaseStateStore, Result as StoreResult, ambiguity_map::AmbiguityCache},
48+
store::{
49+
AvatarCache, BaseStateStore, Result as StoreResult, ambiguity_map::AmbiguityCache,
50+
},
4951
sync::State,
5052
utils::RawStateEventWithKeys,
5153
};
@@ -94,6 +96,7 @@ pub mod sync {
9496
raw_events: Vec<RawStateEventWithKeys<AnySyncStateEvent>>,
9597
room_info: &mut RoomInfo,
9698
ambiguity_cache: &mut AmbiguityCache,
99+
avatar_cache: &mut AvatarCache,
97100
new_users: &mut U,
98101
state_store: &BaseStateStore,
99102
#[cfg(feature = "experimental-encrypted-state-events")] e2ee: &e2ee::E2EE<'_>,
@@ -111,6 +114,7 @@ pub mod sync {
111114
&room_info.room_id,
112115
&mut raw_event,
113116
ambiguity_cache,
117+
avatar_cache,
114118
new_users,
115119
)
116120
.await?;
@@ -177,6 +181,7 @@ pub mod sync {
177181
room_id: &RoomId,
178182
raw_event: &mut RawStateEventWithKeys<AnySyncStateEvent>,
179183
ambiguity_cache: &mut AmbiguityCache,
184+
avatar_cache: &mut AvatarCache,
180185
new_users: &mut U,
181186
) -> StoreResult<()>
182187
where
@@ -189,6 +194,7 @@ pub mod sync {
189194
};
190195

191196
ambiguity_cache.handle_event(&context.state_changes, room_id, event).await?;
197+
avatar_cache.handle_event(&context.state_changes, room_id, event).await?;
192198

193199
match event.membership() {
194200
MembershipState::Join | MembershipState::Invite => {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::{
2929
RequestedRequiredStates,
3030
error::Result,
3131
response_processors as processors,
32-
store::ambiguity_map::AmbiguityCache,
32+
store::{AvatarCache, ambiguity_map::AmbiguityCache},
3333
sync::{RoomUpdates, SyncResponse},
3434
};
3535

@@ -120,6 +120,7 @@ impl BaseClient {
120120

121121
let state_store = self.state_store.clone();
122122
let mut ambiguity_cache = AmbiguityCache::new(state_store.inner.clone());
123+
let mut avatar_cache = AvatarCache::new(state_store.inner.clone());
123124

124125
let global_account_data_processor =
125126
processors::account_data::global(&extensions.account_data.global);
@@ -142,6 +143,7 @@ impl BaseClient {
142143
room_id,
143144
requested_required_states,
144145
&mut ambiguity_cache,
146+
&mut avatar_cache,
145147
),
146148
room_response,
147149
&extensions.account_data.rooms,
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
use std::collections::BTreeMap;
2+
3+
use ruma::{
4+
MxcUri, OwnedMxcUri, OwnedRoomId, OwnedUserId, RoomId, UserId,
5+
events::room::member::SyncRoomMemberEvent,
6+
};
7+
use tracing::trace;
8+
9+
use crate::{StateChanges, StateStore, StoreError, store::SaveLockedStateStore};
10+
11+
/// A cache for keeping track of avatar changes in sync responses.
12+
#[derive(Debug)]
13+
pub struct AvatarCache {
14+
store: SaveLockedStateStore,
15+
changes: BTreeMap<OwnedRoomId, BTreeMap<OwnedUserId, Option<OwnedMxcUri>>>,
16+
}
17+
18+
impl AvatarCache {
19+
/// Creates a new [`AvatarCache`].
20+
pub fn new(store: SaveLockedStateStore) -> Self {
21+
Self { store, changes: BTreeMap::new() }
22+
}
23+
24+
/// Processes the room member event and checks if there was any change in
25+
/// the avatar URL for the room member.
26+
pub async fn handle_event(
27+
&mut self,
28+
state_changes: &StateChanges,
29+
room_id: &RoomId,
30+
member_event: &SyncRoomMemberEvent,
31+
) -> Result<(), StoreError> {
32+
let user_id = member_event.sender();
33+
if self.changes.get(room_id).is_some_and(|user_ids| user_ids.contains_key(user_id)) {
34+
return Ok(());
35+
}
36+
match member_event {
37+
SyncRoomMemberEvent::Original(original_event) => {
38+
let avatar_url = original_event.content.avatar_url.clone();
39+
self.add_to_changes_if_needed(state_changes, room_id, user_id, avatar_url).await;
40+
}
41+
SyncRoomMemberEvent::Redacted(_) => {
42+
trace!("Redacted event, discarding avatar change for {:?}", user_id);
43+
}
44+
}
45+
Ok(())
46+
}
47+
48+
async fn add_to_changes_if_needed(
49+
&mut self,
50+
state_changes: &StateChanges,
51+
room_id: &RoomId,
52+
user_id: &UserId,
53+
avatar: Option<OwnedMxcUri>,
54+
) {
55+
if !self.is_same_avatar(state_changes, room_id, user_id, avatar.as_deref()).await {
56+
trace!("Avatar for {} is different, saving to changes", user_id);
57+
let change = self.changes.entry(room_id.to_owned()).or_default();
58+
change.insert(user_id.to_owned(), avatar);
59+
} else {
60+
trace!("Avatar for {} is the same, not saving", user_id);
61+
}
62+
}
63+
64+
async fn is_same_avatar(
65+
&self,
66+
state_changes: &StateChanges,
67+
room_id: &RoomId,
68+
user_id: &UserId,
69+
avatar: Option<&MxcUri>,
70+
) -> bool {
71+
let current_avatar = if let Some(event) = state_changes.member(room_id, user_id) {
72+
event.content.avatar_url
73+
} else {
74+
match self.store.get_profile(room_id, user_id).await {
75+
Ok(Some(profile)) => profile.content.avatar_url,
76+
Ok(None) => None,
77+
Err(_) => None,
78+
}
79+
};
80+
81+
trace!(
82+
"Current avatar for {} in {} is: {:?}, new avatar is: {:?}",
83+
user_id, room_id, current_avatar, avatar
84+
);
85+
86+
match (current_avatar, avatar) {
87+
(Some(current_avatar), Some(avatar)) => current_avatar == avatar,
88+
(None, None) => true,
89+
_ => false,
90+
}
91+
}
92+
93+
/// Removes and returns the avatar changes associated with the [`RoomId`],
94+
/// if any.
95+
pub fn remove_changes(
96+
&mut self,
97+
room_id: &RoomId,
98+
) -> Option<BTreeMap<OwnedUserId, Option<OwnedMxcUri>>> {
99+
self.changes.remove(room_id)
100+
}
101+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,13 @@ use crate::{
7777
};
7878

7979
pub(crate) mod ambiguity_map;
80+
mod avatar_cache;
8081
mod memory_store;
8182
pub mod migration_helpers;
8283
mod send_queue;
8384

85+
pub use avatar_cache::AvatarCache;
86+
8487
#[cfg(any(test, feature = "testing"))]
8588
pub use self::integration_tests::StateStoreIntegrationTests;
8689
#[cfg(feature = "unstable-msc4274")]

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub use ruma::api::client::sync::sync_events::v3::{
2424
InvitedRoom as InvitedRoomUpdate, KnockedRoom as KnockedRoomUpdate,
2525
};
2626
use ruma::{
27-
OwnedEventId, OwnedRoomId,
27+
OwnedEventId, OwnedMxcUri, OwnedRoomId, OwnedUserId,
2828
api::client::sync::sync_events::UnreadNotificationsCount as RumaUnreadNotificationsCount,
2929
events::{
3030
AnyGlobalAccountDataEvent, AnyRoomAccountDataEvent, AnySyncEphemeralRoomEvent,
@@ -219,6 +219,8 @@ pub struct JoinedRoomUpdate {
219219
/// This is a map of event ID of the `m.room.member` event to the
220220
/// details of the ambiguity change.
221221
pub ambiguity_changes: BTreeMap<OwnedEventId, AmbiguityChange>,
222+
/// Collection of avatar changes that room member events trigger.
223+
pub avatar_changes: Option<BTreeMap<OwnedUserId, Option<OwnedMxcUri>>>,
222224
}
223225

224226
#[cfg(not(tarpaulin_include))]
@@ -243,8 +245,17 @@ impl JoinedRoomUpdate {
243245
ephemeral: Vec<Raw<AnySyncEphemeralRoomEvent>>,
244246
unread_notifications: UnreadNotificationsCount,
245247
ambiguity_changes: BTreeMap<OwnedEventId, AmbiguityChange>,
248+
avatar_changes: Option<BTreeMap<OwnedUserId, Option<OwnedMxcUri>>>,
246249
) -> Self {
247-
Self { unread_notifications, timeline, state, account_data, ephemeral, ambiguity_changes }
250+
Self {
251+
unread_notifications,
252+
timeline,
253+
state,
254+
account_data,
255+
ephemeral,
256+
ambiguity_changes,
257+
avatar_changes,
258+
}
248259
}
249260
}
250261

crates/matrix-sdk-ui/src/timeline/tasks.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,28 @@ pub(in crate::timeline) async fn room_event_cache_updates_task(
273273
timeline_controller.handle_ephemeral_events(events).await;
274274
}
275275

276-
RoomEventCacheUpdate::UpdateMembers { ambiguity_changes } => {
277-
if !ambiguity_changes.is_empty() {
276+
RoomEventCacheUpdate::UpdateMembers { ambiguity_changes, avatar_changes } => {
277+
if !ambiguity_changes.is_empty()
278+
|| !avatar_changes.as_ref().is_none_or(|avatars| avatars.is_empty())
279+
{
278280
let member_ambiguity_changes = ambiguity_changes
279281
.values()
280282
.flat_map(|change| change.user_ids())
281283
.collect::<BTreeSet<_>>();
282-
timeline_controller
283-
.force_update_sender_profiles(&member_ambiguity_changes)
284-
.await;
284+
285+
let mut user_ids_to_update = member_ambiguity_changes;
286+
287+
if let Some(avatar_changes) = &avatar_changes {
288+
let mut user_ids =
289+
avatar_changes.keys().map(|u| u.as_ref()).collect::<BTreeSet<_>>();
290+
user_ids_to_update.append(&mut user_ids)
291+
} else {
292+
warn!(
293+
"No avatar changes to update for {}, ignoring",
294+
room_event_cache.room_id()
295+
);
296+
}
297+
timeline_controller.force_update_sender_profiles(&user_ids_to_update).await;
285298
}
286299
}
287300
}

0 commit comments

Comments
 (0)