mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-07 07:27:45 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat(base): Revisit the roominfo_update API.

Browse Source 
First off, this patch removes the
`RoomInfoNotableUpdate::trigger_room_list_update` field. It is replaced
by a `reasons: RoomInfoNotableUpdateReasons` 8-bit unsigned integer. 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.

Second, 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.

Third, 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.
This commit is contained in:
Ivan Enderlin
2024-07-08 14:28:36 +02:00
parent 66e02f39ef
commit ec7fa76240
8 changed files with 121 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
crates/matrix-sdk-base/src/client.rs
View File

@@ -70,7 +70,10 @@ use crate::RoomMemberships;
use crate::{
deserialized_responses::{RawAnySyncOrStrippedTimelineEvent, SyncTimelineEvent},
error::{Error, Result},
rooms::{normal::RoomInfoNotableUpdate, 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,
@@ -710,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);
}
}
@@ -774,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)
@@ -801,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(())
@@ -1080,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
@@ -1103,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)
@@ -1124,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)
}
}
}
@@ -1229,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(())
}

2
crates/matrix-sdk-base/src/lib.rs
View File

@@ -53,7 +53,7 @@ pub use matrix_sdk_crypto as crypto;
pub use once_cell;
pub use rooms::{
DisplayName, Room, RoomCreateWithCreatorEventContent, RoomHero, RoomInfo,
RoomInfoNotableUpdate, RoomMember, RoomMemberships, RoomState,
RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons, RoomMember, RoomMemberships, RoomState,
RoomStateFilter,
};
pub use store::{

63
crates/matrix-sdk-base/src/rooms/normal.rs
View File

@@ -82,11 +82,27 @@ use crate::{
pub struct RoomInfoNotableUpdate {
/// The room which was updated.
pub room_id: OwnedRoomId,
/// Whether this event should trigger the room list to update.
///
/// If the change is minor or if another action already causes the room list
/// to update, this should be false to avoid duplicate updates.
pub trigger_room_list_update: bool,
/// The reason for this update.
pub reasons: RoomInfoNotableUpdateReasons,
}
bitflags! {
/// The reason why a [`RoomInfoNotableUpdate`] is emitted.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct RoomInfoNotableUpdateReasons: u8 {
/// The recency timestamp of the `Room` has changed.
const RECENCY_TIMESTAMP = 0b0000_0001;
/// The latest event of the `Room` has changed.
const LATEST_EVENT = 0b0000_0010;
}
}
impl Default for RoomInfoNotableUpdateReasons {
fn default() -> Self {
Self::empty()
}
}
/// The underlying room data structure collecting state for joined, left and
@@ -761,16 +777,17 @@ impl Room {
}
/// Update the summary with given RoomInfo.
///
/// This also triggers an update for room info observers if
/// `trigger_room_list_update` is true.
pub fn set_room_info(&self, room_info: RoomInfo, trigger_room_list_update: bool) {
pub fn set_room_info(
&self,
room_info: RoomInfo,
room_info_notable_update_reasons: RoomInfoNotableUpdateReasons,
) {
self.inner.set(room_info);
// Ignore error if no receiver exists.
let _ = self.room_info_notable_update_sender.send(RoomInfoNotableUpdate {
room_id: self.room_id.clone(),
trigger_room_list_update,
reasons: room_info_notable_update_reasons,
});
}
@@ -1594,6 +1611,7 @@ mod tests {
#[cfg(any(feature = "experimental-sliding-sync", feature = "e2e-encryption"))]
use crate::latest_event::LatestEvent;
use crate::{
rooms::normal::RoomInfoNotableUpdateReasons,
store::{MemoryStore, StateChanges, StateStore},
BaseClient, DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, SessionMeta,
};
@@ -1827,7 +1845,7 @@ mod tests {
// When the new tag is handled and applied.
let mut changes = StateChanges::default();
client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await;
client.apply_changes(&changes, false);
client.apply_changes(&changes);
// The `RoomInfo` is getting notified.
assert_ready!(room_info_subscriber);
@@ -1846,7 +1864,7 @@ mod tests {
.unwrap()
.cast();
client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await;
client.apply_changes(&changes, false);
client.apply_changes(&changes);
// The `RoomInfo` is getting notified.
assert_ready!(room_info_subscriber);
@@ -1901,7 +1919,7 @@ mod tests {
// When the new tag is handled and applied.
let mut changes = StateChanges::default();
client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await;
client.apply_changes(&changes, false);
client.apply_changes(&changes);
// The `RoomInfo` is getting notified.
assert_ready!(room_info_subscriber);
@@ -1920,7 +1938,7 @@ mod tests {
.unwrap()
.cast();
client.handle_room_account_data(room_id, &[tag_raw], &mut changes).await;
client.apply_changes(&changes, false);
client.apply_changes(&changes);
// The `RoomInfo` is getting notified.
assert_ready!(room_info_subscriber);
@@ -2341,7 +2359,7 @@ mod tests {
assert_pending!(room_info_subscriber);
// Then updating the room info will store the event,
client.apply_changes(&changes, false);
client.apply_changes(&changes);
assert_eq!(room.latest_event().unwrap().event_id(), event.event_id());
// And wake up the subscriber.
@@ -2362,7 +2380,10 @@ mod tests {
let event = make_latest_event("$A");
let mut changes = StateChanges::default();
room.on_latest_event_decrypted(event.clone(), 0, &mut changes);
room.set_room_info(changes.room_infos.get(room.room_id()).cloned().unwrap(), false);
room.set_room_info(
changes.room_infos.get(room.room_id()).cloned().unwrap(),
RoomInfoNotableUpdateReasons::empty(),
);
// Then is it stored
assert_eq!(room.latest_event().unwrap().event_id(), event.event_id());
@@ -2384,7 +2405,10 @@ mod tests {
let new_event_index = 1;
let mut changes = StateChanges::default();
room.on_latest_event_decrypted(new_event.clone(), new_event_index, &mut changes);
room.set_room_info(changes.room_infos.get(room.room_id()).cloned().unwrap(), false);
room.set_room_info(
changes.room_infos.get(room.room_id()).cloned().unwrap(),
RoomInfoNotableUpdateReasons::empty(),
);
// Then the encrypted events list is shortened to only newer events
let enc_evs = room.latest_encrypted_events();
@@ -2411,7 +2435,10 @@ mod tests {
let new_event_index = 3;
let mut changes = StateChanges::default();
room.on_latest_event_decrypted(new_event, new_event_index, &mut changes);
room.set_room_info(changes.room_infos.get(room.room_id()).cloned().unwrap(), false);
room.set_room_info(
changes.room_infos.get(room.room_id()).cloned().unwrap(),
RoomInfoNotableUpdateReasons::empty(),
);
// Then the encrypted events list ie empty
let enc_evs = room.latest_encrypted_events();

49
crates/matrix-sdk-base/src/sliding_sync.rs
View File

@@ -39,7 +39,10 @@ use crate::RoomMemberships;
use crate::{
error::Result,
read_receipts::{compute_unread_counts, PreviousEventsProvider},
rooms::{normal::RoomHero, RoomState},
rooms::{
normal::{RoomHero, RoomInfoNotableUpdateReasons},
RoomState,
},
store::{ambiguity_map::AmbiguityCache, StateChanges, Store},
sync::{JoinedRoomUpdate, LeftRoomUpdate, Notification, RoomUpdates, SyncResponse},
Room, RoomInfo,
@@ -97,7 +100,7 @@ impl BaseClient {
trace!("ready to submit changes to store");
self.store.save_changes(&changes).await?;
self.apply_changes(&changes, true);
self.apply_changes(&changes);
trace!("applied changes");
Ok(to_device)
@@ -154,8 +157,6 @@ impl BaseClient {
let mut notifications = Default::default();
let mut rooms_account_data = account_data.rooms.clone();
let mut trigger_room_list_update = false;
for (room_id, response_room_data) in rooms {
let (room_info, joined_room, left_room, invited_room) = self
.process_sliding_sync_room(
@@ -166,7 +167,6 @@ impl BaseClient {
&mut changes,
&mut notifications,
&mut ambiguity_cache,
&mut trigger_room_list_update,
)
.await?;
@@ -296,7 +296,7 @@ impl BaseClient {
trace!("ready to submit changes to store");
store.save_changes(&changes).await?;
self.apply_changes(&changes, trigger_room_list_update);
self.apply_changes(&changes);
trace!("applied changes");
// Now that all the rooms information have been saved, update the display name
@@ -326,7 +326,6 @@ impl BaseClient {
changes: &mut StateChanges,
notifications: &mut BTreeMap<OwnedRoomId, Vec<Notification>>,
ambiguity_cache: &mut AmbiguityCache,
trigger_room_list_update: &mut bool,
) -> Result<(RoomInfo, Option<JoinedRoomUpdate>, Option<LeftRoomUpdate>, Option<InvitedRoom>)>
{
let (raw_state_events, state_events): (Vec<_>, Vec<_>) = {
@@ -375,7 +374,7 @@ impl BaseClient {
.await?;
}
process_room_properties(room_data, &mut room_info, trigger_room_list_update);
process_room_properties(room_id, room_data, &mut room_info, changes);
let timeline = self
.handle_timeline(
@@ -691,9 +690,10 @@ async fn cache_latest_events(
}
fn process_room_properties(
room_id: &RoomId,
room_data: &v4::SlidingSyncRoom,
room_info: &mut RoomInfo,
trigger_room_list_update: &mut bool,
changes: &mut StateChanges,
) {
// Handle the room's avatar.
//
@@ -740,7 +740,12 @@ fn process_room_properties(
if let Some(recency_timestamp) = &room_data.timestamp {
room_info.update_recency_timestamp(*recency_timestamp);
*trigger_room_list_update = true;
changes
.room_info_notable_updates
.entry(room_id.to_owned())
.or_default()
.insert(RoomInfoNotableUpdateReasons::RECENCY_TIMESTAMP);
}
}
@@ -778,8 +783,10 @@ mod tests {
use super::cache_latest_events;
use crate::{
rooms::normal::RoomHero, store::MemoryStore, test_utils::logged_in_base_client, BaseClient,
Room, RoomState,
rooms::normal::{RoomHero, RoomInfoNotableUpdateReasons},
store::MemoryStore,
test_utils::logged_in_base_client,
BaseClient, Room, RoomState,
};
#[async_test]
@@ -1557,7 +1564,7 @@ mod tests {
rawev_id(event2.clone())
);
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
assert_eq!(
ev_id(room.latest_event().map(|latest_event| latest_event.event().clone())),
rawev_id(event2)
@@ -1579,7 +1586,7 @@ mod tests {
let room = make_room();
let mut room_info = room.clone_info();
cache_latest_events(&room, &mut room_info, events, None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// The latest message is stored
assert_eq!(
@@ -1606,7 +1613,7 @@ mod tests {
let room = make_room();
let mut room_info = room.clone_info();
cache_latest_events(&room, &mut room_info, events, None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// The latest message is stored, ignoring the receipt
assert_eq!(
@@ -1659,7 +1666,7 @@ mod tests {
let room = make_room();
let mut room_info = room.clone_info();
cache_latest_events(&room, &mut room_info, events, None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// The latest message is stored, ignoring encrypted and receipts
assert_eq!(
@@ -1700,7 +1707,7 @@ mod tests {
None,
)
.await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// Sanity: room_info has 10 encrypted events inside it
assert_eq!(room.latest_encrypted_events.read().unwrap().len(), 10);
@@ -1709,7 +1716,7 @@ mod tests {
let eventa = make_encrypted_event("$a");
let mut room_info = room.clone_info();
cache_latest_events(&room, &mut room_info, &[eventa], None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// The oldest event is gone
assert!(!rawevs_ids(&room.latest_encrypted_events).contains(&"$0".to_owned()));
@@ -1731,13 +1738,13 @@ mod tests {
None,
)
.await;
room.set_room_info(room_info.clone(), false);
room.set_room_info(room_info.clone(), RoomInfoNotableUpdateReasons::empty());
// When I ask to cache an unencrypted event, and some more encrypted events
let eventa = make_event("m.room.message", "$a");
let eventb = make_encrypted_event("$b");
cache_latest_events(&room, &mut room_info, &[eventa, eventb], None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
// The only encrypted events stored are the ones after the decrypted one
assert_eq!(rawevs_ids(&room.latest_encrypted_events), &["$b"]);
@@ -1824,7 +1831,7 @@ mod tests {
let room = make_room();
let mut room_info = room.clone_info();
cache_latest_events(&room, &mut room_info, events, None, None).await;
room.set_room_info(room_info, false);
room.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
room.latest_event().map(|latest_event| latest_event.event().clone())
}

17
crates/matrix-sdk-base/src/store/mod.rs
View File

@@ -60,7 +60,7 @@ use tokio::sync::{broadcast, Mutex, RwLock};
use crate::{
rooms::{normal::RoomInfoNotableUpdate, RoomInfo, RoomState},
MinimalRoomMemberEvent, Room, RoomStateFilter, SessionMeta,
MinimalRoomMemberEvent, Room, RoomInfoNotableUpdateReasons, RoomStateFilter, SessionMeta,
};
pub(crate) mod ambiguity_map;
@@ -313,8 +313,21 @@ pub struct StateChanges {
/// A mapping of `RoomId` to a map of event type string to `AnyBasicEvent`.
pub room_account_data:
BTreeMap<OwnedRoomId, BTreeMap<RoomAccountDataEventType, Raw<AnyRoomAccountDataEvent>>>,
/// A map of `RoomId` to `RoomInfo`.
/// A map of `OwnedRoomId` to `RoomInfo`.
pub room_infos: BTreeMap<OwnedRoomId, RoomInfo>,
/// A map of `OwnedRoomId` to `RoomInfoNotableUpdateReason`.
///
/// A room info notable update is an update that can be interested for other
/// parts of the code. This mechanism is used in coordination with
/// [`BaseClient::room_info_notable_update_receiver`][baseclient] where
/// `RoomInfo` can be observed and some of its updates can be spread to
/// listeners.
///
/// [baseclient]: crate::BaseClient::room_info_notable_update_receiver
pub room_info_notable_updates: BTreeMap<OwnedRoomId, RoomInfoNotableUpdateReasons>,
/// A map of `RoomId` to `ReceiptEventContent`.
pub receipts: BTreeMap<OwnedRoomId, ReceiptEventContent>,

1
crates/matrix-sdk-sqlite/src/state_store.rs
View File

@@ -992,6 +992,7 @@ impl StateStore for SqliteStateStore {
state,
room_account_data,
room_infos,
room_info_notable_updates: _,
receipts,
redactions,
stripped_state,

14
crates/matrix-sdk-ui/src/room_list_service/room_list.rs
View File

@@ -25,7 +25,7 @@ use matrix_sdk::{
executor::{spawn, JoinHandle},
Client, SlidingSync, SlidingSyncList,
};
use matrix_sdk_base::RoomInfoNotableUpdate;
use matrix_sdk_base::{RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons};
use tokio::{select, sync::broadcast};
use super::{
@@ -214,10 +214,14 @@ fn merge_stream_and_receiver(
Ok(update) = room_info_notable_update_receiver.recv() => {
let reasons = &update.reasons;
// Search list for the updated room
if let Some(index) = raw_current_values.iter().position(|room| room.room_id() == update.room_id) {
let update = VectorDiff::Set { index, value: raw_current_values[index].clone() };
yield vec![update];
// We are interested by these _reasons_.
if reasons.contains(RoomInfoNotableUpdateReasons::LATEST_EVENT) ||
reasons.contains(RoomInfoNotableUpdateReasons::RECENCY_TIMESTAMP) {
// Emit a `VectorDiff::Set` for the specific rooms.
if let Some(index) = raw_current_values.iter().position(|room| room.room_id() == update.room_id) {
let update = VectorDiff::Set { index, value: raw_current_values[index].clone() };
yield vec![update];
}
}
}
}

5
crates/matrix-sdk/src/room/mod.rs
View File

@@ -20,7 +20,8 @@ use matrix_sdk_base::{
},
instant::Instant,
store::StateStoreExt,
ComposerDraft, RoomMemberships, StateChanges, StateStoreDataKey, StateStoreDataValue,
ComposerDraft, RoomInfoNotableUpdateReasons, RoomMemberships, StateChanges, StateStoreDataKey,
StateStoreDataValue,
};
use matrix_sdk_common::timeout::timeout;
use mime::Mime;
@@ -495,7 +496,7 @@ impl Room {
changes.add_room(room_info.clone());
self.client.store().save_changes(&changes).await?;
self.set_room_info(room_info, false);
self.set_room_info(room_info, RoomInfoNotableUpdateReasons::empty());
Ok(())
})