From 6cd655ba7c817d3e650f9b8ea6124d48c6e8ce88 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 29 Mar 2024 14:09:38 +0100 Subject: [PATCH] state store: add `Changes::profile_to_delete` field So as to remove existing profiles from the storage. --- .../src/store/integration_tests.rs | 83 ++++++++++++++++++- .../matrix-sdk-base/src/store/memory_store.rs | 27 ++++-- crates/matrix-sdk-base/src/store/mod.rs | 5 ++ .../src/state_store/mod.rs | 12 ++- crates/matrix-sdk-sqlite/src/state_store.rs | 16 ++++ 5 files changed, 132 insertions(+), 11 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/integration_tests.rs b/crates/matrix-sdk-base/src/store/integration_tests.rs index 345c002d7..bdb01cf90 100644 --- a/crates/matrix-sdk-base/src/store/integration_tests.rs +++ b/crates/matrix-sdk-base/src/store/integration_tests.rs @@ -41,8 +41,8 @@ use crate::{ /// `StateStore` integration tests. /// -/// This trait is not meant to be used directly, but will be used with the [``] -/// macro. +/// This trait is not meant to be used directly, but will be used with the +/// [`statestore_integration_tests!`] macro. #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] #[cfg_attr(not(target_arch = "wasm32"), async_trait)] pub trait StateStoreIntegrationTests { @@ -74,6 +74,8 @@ pub trait StateStoreIntegrationTests { async fn test_stripped_non_stripped(&self) -> Result<()>; /// Test room removal. async fn test_room_removal(&self) -> Result<()>; + /// Test profile removal. + async fn test_profile_removal(&self) -> Result<()>; /// Test presence saving. async fn test_presence_saving(&self); /// Test display names saving. @@ -1058,6 +1060,77 @@ impl StateStoreIntegrationTests for DynStateStore { Ok(()) } + async fn test_profile_removal(&self) -> Result<()> { + let room_id = room_id(); + + // Both the user id and invited user id get a profile in populate(). + let user_id = user_id(); + let invited_user_id = invited_user_id(); + + self.populate().await?; + + let new_invite_member_json = json!({ + "content": { + "avatar_url": "mxc://localhost/SEsfnsuifSDFSSEG", + "displayname": "example after update", + "membership": "invite", + "reason": "Looking for support" + }, + "event_id": "$143273582443PhrSm:localhost", + "origin_server_ts": 1432735824, + "room_id": room_id, + "sender": user_id, + "state_key": invited_user_id, + "type": "m.room.member", + }); + let new_invite_member_event: SyncRoomMemberEvent = + serde_json::from_value(new_invite_member_json.clone()).unwrap(); + + let mut changes = StateChanges { + // Both get their profiles deleted… + profiles_to_delete: [( + room_id.to_owned(), + vec![user_id.to_owned(), invited_user_id.to_owned()], + )] + .into(), + + // …but the invited user get a new profile. + profiles: { + let mut map = BTreeMap::default(); + map.insert( + room_id.to_owned(), + [(invited_user_id.to_owned(), new_invite_member_event.into())] + .into_iter() + .collect(), + ); + map + }, + + ..StateChanges::default() + }; + + let raw = serde_json::from_value::>(new_invite_member_json) + .expect("can create sync-state-event for topic"); + let event = raw.deserialize().unwrap(); + changes.add_state_event(room_id, event, raw); + + self.save_changes(&changes).await.unwrap(); + + // The profile for user has been removed. + assert!(self.get_profile(room_id, user_id).await?.is_none()); + assert!(self.get_member_event(room_id, user_id).await?.is_some()); + + // The profile for the invited user has been updated. + let invited_member_event = self.get_profile(room_id, invited_user_id).await?.unwrap(); + assert_eq!( + invited_member_event.as_original().unwrap().content.displayname.as_deref(), + Some("example after update") + ); + assert!(self.get_member_event(room_id, invited_user_id).await?.is_some()); + + Ok(()) + } + async fn test_presence_saving(&self) { let user_id = user_id(); let second_user_id = user_id!("@second:localhost"); @@ -1288,6 +1361,12 @@ macro_rules! statestore_integration_tests { store.test_room_removal().await } + #[async_test] + async fn test_profile_removal() -> StoreResult<()> { + let store = get_store().await?.into_state_store(); + store.test_profile_removal().await + } + #[async_test] async fn test_presence_saving() { let store = get_store().await.expect("creating store failed").into_state_store(); diff --git a/crates/matrix-sdk-base/src/store/memory_store.rs b/crates/matrix-sdk-base/src/store/memory_store.rs index 065cacfd3..f2c1c635e 100644 --- a/crates/matrix-sdk-base/src/store/memory_store.rs +++ b/crates/matrix-sdk-base/src/store/memory_store.rs @@ -227,14 +227,25 @@ impl StateStore for MemoryStore { *self.sync_token.write().unwrap() = Some(s.to_owned()); } - for (room, users) in &changes.profiles { - for (user_id, profile) in users { - self.profiles - .write() - .unwrap() - .entry(room.clone()) - .or_default() - .insert(user_id.clone(), profile.clone()); + { + let mut profiles = self.profiles.write().unwrap(); + + for (room, users) in &changes.profiles_to_delete { + let Some(room_profiles) = profiles.get_mut(room) else { + continue; + }; + for user in users { + room_profiles.remove(user); + } + } + + for (room, users) in &changes.profiles { + for (user_id, profile) in users { + profiles + .entry(room.clone()) + .or_default() + .insert(user_id.clone(), profile.clone()); + } } } diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index 586c07e4a..f278ad410 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -277,6 +277,11 @@ pub struct StateChanges { /// `MinimalRoomMemberEvent`. pub profiles: BTreeMap>, + /// A mapping of room profiles to delete. + /// + /// These are deleted *before* other room profiles are inserted. + pub profiles_to_delete: BTreeMap>, + /// A mapping of `RoomId` to a map of event type string to a state key and /// `AnySyncStateEvent`. pub state: diff --git a/crates/matrix-sdk-indexeddb/src/state_store/mod.rs b/crates/matrix-sdk-indexeddb/src/state_store/mod.rs index 9b0b55940..642b8b230 100644 --- a/crates/matrix-sdk-indexeddb/src/state_store/mod.rs +++ b/crates/matrix-sdk-indexeddb/src/state_store/mod.rs @@ -489,7 +489,10 @@ impl_state_store!({ (!changes.ambiguity_maps.is_empty(), keys::DISPLAY_NAMES), (!changes.account_data.is_empty(), keys::ACCOUNT_DATA), (!changes.presence.is_empty(), keys::PRESENCE), - (!changes.profiles.is_empty(), keys::PROFILES), + ( + !changes.profiles.is_empty() || !changes.profiles_to_delete.is_empty(), + keys::PROFILES, + ), (!changes.room_account_data.is_empty(), keys::ROOM_ACCOUNT_DATA), (!changes.receipts.is_empty(), keys::ROOM_EVENT_RECEIPTS), ] @@ -577,6 +580,13 @@ impl_state_store!({ let stripped_state = tx.object_store(keys::STRIPPED_ROOM_STATE)?; let stripped_user_ids = tx.object_store(keys::STRIPPED_USER_IDS)?; + for (room, user_ids) in &changes.profiles_to_delete { + for user_id in user_ids { + let key = self.encode_key(keys::PROFILES, (room, user_id)); + profiles.delete(&key)?; + } + } + for (room, event_types) in &changes.state { let profile_changes = changes.profiles.get(room); diff --git a/crates/matrix-sdk-sqlite/src/state_store.rs b/crates/matrix-sdk-sqlite/src/state_store.rs index 211317c3b..227732e0a 100644 --- a/crates/matrix-sdk-sqlite/src/state_store.rs +++ b/crates/matrix-sdk-sqlite/src/state_store.rs @@ -378,6 +378,7 @@ trait SqliteConnectionStateStoreExt { fn set_profile(&self, room_id: &[u8], user_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; fn remove_room_profiles(&self, room_id: &[u8]) -> rusqlite::Result<()>; + fn remove_room_profile(&self, room_id: &[u8], user_id: &[u8]) -> rusqlite::Result<()>; fn set_receipt( &self, @@ -550,6 +551,12 @@ impl SqliteConnectionStateStoreExt for rusqlite::Connection { Ok(()) } + fn remove_room_profile(&self, room_id: &[u8], user_id: &[u8]) -> rusqlite::Result<()> { + self.prepare("DELETE FROM profile WHERE room_id = ? AND user_id = ?")? + .execute((room_id, user_id))?; + Ok(()) + } + fn set_receipt( &self, room_id: &[u8], @@ -921,6 +928,7 @@ impl StateStore for SqliteStateStore { account_data, presence, profiles, + profiles_to_delete, state, room_account_data, room_infos, @@ -971,6 +979,14 @@ impl StateStore for SqliteStateStore { txn.set_room_info(&room_id, &state, &data)?; } + for (room_id, user_ids) in profiles_to_delete { + let room_id = this.encode_key(keys::PROFILE, room_id); + for user_id in user_ids { + let user_id = this.encode_key(keys::PROFILE, user_id); + txn.remove_room_profile(&room_id, &user_id)?; + } + } + for (room_id, state_event_types) in state { let profiles = profiles.get(&room_id); let encoded_room_id = this.encode_key(keys::STATE_EVENT, &room_id);