Skip to content

Commit

Permalink
feat(base): Revisit the roominfo_update API.
Browse files Browse the repository at this point in the history
This patch updates several parts of the code but it's basically just
renamings.

First off, the patch renames `RoomInfoUpdate` to
`RoomInfoNotableUpdate`. The functions, methods or variables
whose names start with `roominfo_update(.*)` are renamed
`room_info_notable_update$1`.

Second, this patch removes the
`RoomInfoNotableUpdate::trigger_room_list_update` field. It is replaced
by a `reasons: RoomInfoNotableUpdateReasons` 8-bit unsigned integer,
represented as an `enum` type. It addresses the following issues:

1. When a subscriber receives a `RoomInfoNotableUpdate`, they have no
   idea what has triggered this update.
2. In
   `matrix_sdk_base::sliding_sync::BaseClient::process_sliding_sync_e2ee`,
   we were triggering an update even if the latest event wasn't
   modified: it is a false-positive, it was a bug and a waste of
   resources. Now it's more refined, see the why below.

Third, this patch removes the second `trigger_room_list_update` argument
of `matrix_sdk_base::BaseClient::apply_changes`. This method now knows
where to find the reasons for the room info notable updates, see next
point.

Fourth, this patch adds a new
`matrix_sdk::StateChanges::room_info_notable_updates` field which is a
B-tree map between an `OwnedRoomId` and a
`RoomInfoNotableUpdateReasons`. The idea is that all places that receive
a `StateChanges` can also create a room info notable update with a
specific reason. This is a finer grained mechanism, and it avoids to
pass new arguments everywhere. It's cleaner.

Finally, it's easier than ever to add a new reason and to propagate it
to subscribers.
  • Loading branch information
Hywan committed Jul 3, 2024
1 parent 5bbd94b commit 6709a6b
Show file tree
Hide file tree
Showing 14 changed files with 191 additions and 112 deletions.
2 changes: 1 addition & 1 deletion bindings/matrix-sdk-ffi/src/room_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl RoomList {
let (entries_stream, dynamic_entries_controller) =
this.inner.entries_with_dynamic_adapters(
page_size.try_into().unwrap(),
client.roominfo_update_receiver(),
client.room_info_notable_update_receiver(),
);

// FFI dance to make those values consumable by foreign language, nothing fancy
Expand Down
65 changes: 40 additions & 25 deletions crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ use crate::RoomMemberships;
use crate::{
deserialized_responses::{RawAnySyncOrStrippedTimelineEvent, SyncTimelineEvent},
error::{Error, Result},
rooms::{normal::RoomInfoUpdate, Room, RoomInfo, RoomState},
rooms::{
normal::{RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons},
Room, RoomInfo, RoomState,
},
store::{
ambiguity_map::AmbiguityCache, DynStateStore, MemoryStore, Result as StoreResult,
StateChanges, StateStoreDataKey, StateStoreDataValue, StateStoreExt, Store, StoreConfig,
Expand Down Expand Up @@ -104,7 +107,7 @@ pub struct BaseClient {
/// A sender that is used to communicate changes to room information. Each
/// event contains the room and a boolean whether this event should
/// trigger a room list update.
pub(crate) roominfo_update_sender: broadcast::Sender<RoomInfoUpdate>,
pub(crate) room_info_notable_update_sender: broadcast::Sender<RoomInfoNotableUpdate>,
}

#[cfg(not(tarpaulin_include))]
Expand All @@ -130,7 +133,8 @@ impl BaseClient {
/// * `config` - An optional session if the user already has one from a
/// previous login call.
pub fn with_store_config(config: StoreConfig) -> Self {
let (roominfo_update_sender, _roominfo_update_receiver) = broadcast::channel(100);
let (room_info_notable_update_sender, _room_info_notable_update_receiver) =
broadcast::channel(100);

BaseClient {
store: Store::new(config.state_store),
Expand All @@ -139,7 +143,7 @@ impl BaseClient {
#[cfg(feature = "e2e-encryption")]
olm_machine: Default::default(),
ignore_user_list_changes: Default::default(),
roominfo_update_sender,
room_info_notable_update_sender,
}
}

Expand Down Expand Up @@ -183,7 +187,11 @@ impl BaseClient {
/// Lookup the Room for the given RoomId, or create one, if it didn't exist
/// yet in the store
pub fn get_or_create_room(&self, room_id: &RoomId, room_state: RoomState) -> Room {
self.store.get_or_create_room(room_id, room_state, self.roominfo_update_sender.clone())
self.store.get_or_create_room(
room_id,
room_state,
self.room_info_notable_update_sender.clone(),
)
}

/// Get a reference to the store.
Expand Down Expand Up @@ -225,7 +233,9 @@ impl BaseClient {
>,
) -> Result<()> {
debug!(user_id = ?session_meta.user_id, device_id = ?session_meta.device_id, "Restoring login");
self.store.set_session_meta(session_meta.clone(), &self.roominfo_update_sender).await?;
self.store
.set_session_meta(session_meta.clone(), &self.room_info_notable_update_sender)
.await?;

#[cfg(feature = "e2e-encryption")]
self.regenerate_olm(custom_account).await?;
Expand Down Expand Up @@ -703,6 +713,12 @@ impl BaseClient {
// encrypted events
if let Some((found, found_index)) = self.decrypt_latest_suitable_event(room).await {
room.on_latest_event_decrypted(found, found_index, changes);

changes
.room_info_notable_updates
.entry(room.room_id().to_owned())
.or_default()
.insert(RoomInfoNotableUpdateReasons::LATEST_EVENT);
}
}

Expand Down Expand Up @@ -754,7 +770,7 @@ impl BaseClient {
let room = self.store.get_or_create_room(
room_id,
RoomState::Joined,
self.roominfo_update_sender.clone(),
self.room_info_notable_update_sender.clone(),
);

if room.state() != RoomState::Joined {
Expand All @@ -767,8 +783,7 @@ impl BaseClient {
let mut changes = StateChanges::default();
changes.add_room(room_info.clone());
self.store.save_changes(&changes).await?; // Update the store
room.set_room_info(room_info, false); // Update the cached room
// handle
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
}

Ok(room)
Expand All @@ -781,7 +796,7 @@ impl BaseClient {
let room = self.store.get_or_create_room(
room_id,
RoomState::Left,
self.roominfo_update_sender.clone(),
self.room_info_notable_update_sender.clone(),
);

if room.state() != RoomState::Left {
Expand All @@ -794,8 +809,7 @@ impl BaseClient {
let mut changes = StateChanges::default();
changes.add_room(room_info.clone());
self.store.save_changes(&changes).await?; // Update the store
room.set_room_info(room_info, false); // Update the cached room
// handle
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
}

Ok(())
Expand Down Expand Up @@ -857,7 +871,7 @@ impl BaseClient {
let room = self.store.get_or_create_room(
&room_id,
RoomState::Joined,
self.roominfo_update_sender.clone(),
self.room_info_notable_update_sender.clone(),
);

let mut room_info = room.clone_info();
Expand Down Expand Up @@ -970,7 +984,7 @@ impl BaseClient {
let room = self.store.get_or_create_room(
&room_id,
RoomState::Left,
self.roominfo_update_sender.clone(),
self.room_info_notable_update_sender.clone(),
);

let mut room_info = room.clone_info();
Expand Down Expand Up @@ -1029,7 +1043,7 @@ impl BaseClient {
let room = self.store.get_or_create_room(
&room_id,
RoomState::Invited,
self.roominfo_update_sender.clone(),
self.room_info_notable_update_sender.clone(),
);

let mut room_info = room.clone_info();
Expand Down Expand Up @@ -1073,7 +1087,7 @@ impl BaseClient {
let _sync_lock = self.sync_lock().lock().await;
self.store.save_changes(&changes).await?;
*self.store.sync_token.write().await = Some(response.next_batch.clone());
self.apply_changes(&changes, false);
self.apply_changes(&changes);
}

// Now that all the rooms information have been saved, update the display name
Expand All @@ -1096,7 +1110,7 @@ impl BaseClient {
Ok(response)
}

pub(crate) fn apply_changes(&self, changes: &StateChanges, trigger_room_list_update: bool) {
pub(crate) fn apply_changes(&self, changes: &StateChanges) {
if changes.account_data.contains_key(&GlobalAccountDataEventType::IgnoredUserList) {
if let Some(event) =
changes.account_data.get(&GlobalAccountDataEventType::IgnoredUserList)
Expand All @@ -1117,7 +1131,10 @@ impl BaseClient {

for (room_id, room_info) in &changes.room_infos {
if let Some(room) = self.store.room(room_id) {
room.set_room_info(room_info.clone(), trigger_room_list_update)
let room_info_notable_update_reasons =
changes.room_info_notable_updates.get(room_id).copied().unwrap_or_default();

room.set_room_info(room_info.clone(), room_info_notable_update_reasons)
}
}
}
Expand Down Expand Up @@ -1222,7 +1239,7 @@ impl BaseClient {
changes.add_room(room_info);

self.store.save_changes(&changes).await?;
self.apply_changes(&changes, false);
self.apply_changes(&changes);

Ok(())
}
Expand Down Expand Up @@ -1476,13 +1493,11 @@ impl BaseClient {
.collect()
}

/// Returns a new receiver that gets events for all future room info
/// updates.
/// Returns a new receiver that gets future room info notable updates.
///
/// Each event contains the room and a boolean whether this event should
/// trigger a room list update.
pub fn roominfo_update_receiver(&self) -> broadcast::Receiver<RoomInfoUpdate> {
self.roominfo_update_sender.subscribe()
/// Learn more by reading the [`RoomInfoNotableUpdate`] type.
pub fn room_info_notable_update_receiver(&self) -> broadcast::Receiver<RoomInfoNotableUpdate> {
self.room_info_notable_update_sender.subscribe()
}
}

Expand Down
5 changes: 3 additions & 2 deletions crates/matrix-sdk-base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ pub use http;
pub use matrix_sdk_crypto as crypto;
pub use once_cell;
pub use rooms::{
DisplayName, Room, RoomCreateWithCreatorEventContent, RoomHero, RoomInfo, RoomInfoUpdate,
RoomMember, RoomMemberships, RoomState, RoomStateFilter,
DisplayName, Room, RoomCreateWithCreatorEventContent, RoomHero, RoomInfo,
RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMember, RoomMemberships, RoomState,
RoomStateFilter,
};
pub use store::{
ComposerDraft, ComposerDraftType, StateChanges, StateStore, StateStoreDataKey,
Expand Down
5 changes: 4 additions & 1 deletion crates/matrix-sdk-base/src/rooms/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use std::{

use bitflags::bitflags;
pub use members::RoomMember;
pub use normal::{Room, RoomHero, RoomInfo, RoomInfoUpdate, RoomState, RoomStateFilter};
pub use normal::{
Room, RoomHero, RoomInfo, RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomState,
RoomStateFilter,
};
use ruma::{
assign,
events::{
Expand Down
Loading

0 comments on commit 6709a6b

Please sign in to comment.