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.

Sign up for GitHub

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

sdk-base: split handle_account_data and process_direct_rooms #4010

Merged
merged 1 commit into from
Sep 18, 2024
Merged

sdk-base: split handle_account_data and process_direct_rooms #4010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
135 changes: 78 additions & 57 deletions crates/matrix-sdk-base/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,13 +688,18 @@ impl BaseClient {
}
}

/// Parses and stores any raw global account data events into the
/// [`StateChanges`].
///
/// Returns a list with the parsed account data events.
#[instrument(skip_all)]
pub(crate) async fn handle_account_data(
&self,
events: &[Raw<AnyGlobalAccountDataEvent>],
changes: &mut StateChanges,
) {
) -> Vec<AnyGlobalAccountDataEvent> {
let mut account_data = BTreeMap::new();
let mut parsed_events = Vec::new();

for raw_event in events {
let event = match raw_event.deserialize() {
Expand All @@ -705,64 +710,78 @@ impl BaseClient {
continue;
}
};
account_data.insert(event.event_type(), raw_event.clone());
parsed_events.push(event);
}

if let AnyGlobalAccountDataEvent::Direct(e) = &event {
let mut new_dms = HashMap::<&RoomId, HashSet<OwnedUserId>>::new();
for (user_id, rooms) in e.content.iter() {
for room_id in rooms {
new_dms.entry(room_id).or_default().insert(user_id.clone());
}
changes.account_data = account_data;
parsed_events
}

/// Processes the direct rooms in a sync response:
///
/// Given a [`StateChanges`] instance, processes any direct room info
/// from the global account data and adds it to the room infos to
/// save.
#[instrument(skip_all)]
pub(crate) async fn process_direct_rooms(
&self,
events: &[AnyGlobalAccountDataEvent],
changes: &mut StateChanges,
) {
for event in events {
let AnyGlobalAccountDataEvent::Direct(direct_event) = event else { continue };
let mut new_dms = HashMap::<&RoomId, HashSet<OwnedUserId>>::new();
for (user_id, rooms) in direct_event.content.iter() {
for room_id in rooms {
new_dms.entry(room_id).or_default().insert(user_id.clone());
}
}

let rooms = self.store.rooms();
let mut old_dms = rooms
.iter()
.filter_map(|r| {
let direct_targets = r.direct_targets();
(!direct_targets.is_empty()).then(|| (r.room_id(), direct_targets))
})
.collect::<HashMap<_, _>>();

// Update the direct targets of rooms if they changed.
for (room_id, new_direct_targets) in new_dms {
if let Some(old_direct_targets) = old_dms.remove(&room_id) {
if old_direct_targets == new_direct_targets {
continue;
}
let rooms = self.store.rooms();
let mut old_dms = rooms
.iter()
.filter_map(|r| {
let direct_targets = r.direct_targets();
(!direct_targets.is_empty()).then(|| (r.room_id(), direct_targets))
})
.collect::<HashMap<_, _>>();

// Update the direct targets of rooms if they changed.
for (room_id, new_direct_targets) in new_dms {
if let Some(old_direct_targets) = old_dms.remove(&room_id) {
if old_direct_targets == new_direct_targets {
continue;
}
}

trace!(
?room_id, targets = ?new_direct_targets,
"Marking room as direct room"
);
trace!(
?room_id, targets = ?new_direct_targets,
"Marking room as direct room"
);

if let Some(info) = changes.room_infos.get_mut(room_id) {
info.base_info.dm_targets = new_direct_targets;
} else if let Some(room) = self.store.room(room_id) {
let mut info = room.clone_info();
info.base_info.dm_targets = new_direct_targets;
changes.add_room(info);
}
if let Some(info) = changes.room_infos.get_mut(room_id) {
info.base_info.dm_targets = new_direct_targets;
} else if let Some(room) = self.store.room(room_id) {
let mut info = room.clone_info();
info.base_info.dm_targets = new_direct_targets;
changes.add_room(info);
}
}

// Remove the targets of old direct chats.
for room_id in old_dms.keys() {
trace!(?room_id, "Unmarking room as direct room");
// Remove the targets of old direct chats.
for room_id in old_dms.keys() {
trace!(?room_id, "Unmarking room as direct room");

if let Some(info) = changes.room_infos.get_mut(*room_id) {
info.base_info.dm_targets.clear();
} else if let Some(room) = self.store.room(room_id) {
let mut info = room.clone_info();
info.base_info.dm_targets.clear();
changes.add_room(info);
}
if let Some(info) = changes.room_infos.get_mut(*room_id) {
info.base_info.dm_targets.clear();
} else if let Some(room) = self.store.room(room_id) {
let mut info = room.clone_info();
info.base_info.dm_targets.clear();
changes.add_room(info);
}
}

account_data.insert(event.event_type(), raw_event.clone());
}

changes.account_data = account_data;
}

#[cfg(feature = "e2e-encryption")]
Expand Down Expand Up @@ -971,7 +990,8 @@ impl BaseClient {

let mut ambiguity_cache = AmbiguityCache::new(self.store.inner.clone());

self.handle_account_data(&response.account_data.events, &mut changes).await;
let global_account_data_events =
self.handle_account_data(&response.account_data.events, &mut changes).await;

let push_rules = self.get_push_rules(&changes).await?;

Expand Down Expand Up @@ -1186,23 +1206,24 @@ impl BaseClient {
new_rooms.invite.insert(room_id, new_info);
}

// TODO remove this, we're processing account data events here again
// We're processing direct state events here separately
// because we want to have the push rules in place before we process
// rooms and their events, but we want to create the rooms before we
// process the `m.direct` account data event.
let has_new_direct_room_data = response.account_data.events.iter().any(|raw_event| {
raw_event
.deserialize()
.map(|event| event.event_type() == GlobalAccountDataEventType::Direct)
.unwrap_or_default()
});
let has_new_direct_room_data = global_account_data_events
.iter()
.any(|event| event.event_type() == GlobalAccountDataEventType::Direct);
if has_new_direct_room_data {
self.handle_account_data(&response.account_data.events, &mut changes).await;
self.process_direct_rooms(&global_account_data_events, &mut changes).await;
} else if let Ok(Some(direct_account_data)) =
self.store.get_account_data_event(GlobalAccountDataEventType::Direct).await
{
debug!("Found direct room data in the Store, applying it");
self.handle_account_data(&[direct_account_data], &mut changes).await;
if let Ok(direct_account_data) = direct_account_data.deserialize() {
self.process_direct_rooms(&[direct_account_data], &mut changes).await;
} else {
warn!("Failed to deserialize direct room account data");
}
}

changes.presence = response
Expand Down
27 changes: 15 additions & 12 deletions crates/matrix-sdk-base/src/sliding_sync/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,11 @@ impl BaseClient {
let mut ambiguity_cache = AmbiguityCache::new(store.inner.clone());

let account_data = &extensions.account_data;
if !account_data.is_empty() {
self.handle_account_data(&account_data.global, &mut changes).await;
}
let global_account_data_events = if !account_data.is_empty() {
self.handle_account_data(&account_data.global, &mut changes).await
} else {
Vec::new()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Here this is fine to get a Vec::new(), it does zero allocation.

I know it's different from what we've talked in the above comments, but I'm sharing some Rust knowledge.

};

let mut new_rooms = RoomUpdates::default();
let mut notifications = Default::default();
Expand Down Expand Up @@ -302,23 +304,24 @@ impl BaseClient {
}
}

// TODO remove this, we're processing account data events here again
// We're processing direct state events here separately
// because we want to have the push rules in place before we process
// rooms and their events, but we want to create the rooms before we
// process the `m.direct` account data event.
let has_new_direct_room_data = account_data.global.iter().any(|raw_event| {
raw_event
.deserialize()
.map(|event| event.event_type() == GlobalAccountDataEventType::Direct)
.unwrap_or_default()
});
let has_new_direct_room_data = global_account_data_events
.iter()
.any(|event| event.event_type() == GlobalAccountDataEventType::Direct);
if has_new_direct_room_data {
self.handle_account_data(&account_data.global, &mut changes).await;
self.process_direct_rooms(&global_account_data_events, &mut changes).await;
} else if let Ok(Some(direct_account_data)) =
self.store.get_account_data_event(GlobalAccountDataEventType::Direct).await
{
debug!("Found direct room data in the Store, applying it");
self.handle_account_data(&[direct_account_data], &mut changes).await;
if let Ok(direct_account_data) = direct_account_data.deserialize() {
self.process_direct_rooms(&[direct_account_data], &mut changes).await;
} else {
warn!("Failed to deserialize direct room account data");
}
}

// FIXME not yet supported by sliding sync.
Expand Down
Loading