From 4fc21a8860265063d2dcf0e20299c16a9901ab54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 28 Apr 2021 16:46:58 +0200 Subject: [PATCH] base: Store the raw versions of events in the state store This patch changes the way we store and load the majority of events that are in the state store. This is done so custom fields in the event aren't lost since a deserialize/serialize cycle removes all the unknown fields from the event. --- matrix_sdk_base/src/client.rs | 78 +++++++++--------- matrix_sdk_base/src/rooms/normal.rs | 7 +- matrix_sdk_base/src/store/memory_store.rs | 47 ++++++----- matrix_sdk_base/src/store/mod.rs | 62 ++++++++------- matrix_sdk_base/src/store/sled_store/mod.rs | 87 +++++++++++++++------ 5 files changed, 174 insertions(+), 107 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 60102b2d..528bced4 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -473,7 +473,9 @@ impl BaseClient { } _ => { room_info.handle_state_event(&s.content()); - changes.add_state_event(room_id, s.clone()); + let raw_event: Raw = + Raw::from_json(event.event.clone().into_json()); + changes.add_state_event(room_id, s.clone(), raw_event); } }, @@ -543,12 +545,12 @@ impl BaseClient { room_info: &mut RoomInfo, ) -> ( BTreeMap, - BTreeMap>, + BTreeMap>>, ) { events.iter().fold( (BTreeMap::new(), BTreeMap::new()), - |(mut members, mut state_events), e| { - match e.deserialize() { + |(mut members, mut state_events), raw_event| { + match raw_event.deserialize() { Ok(e) => { if let AnyStrippedStateEvent::RoomMember(member) = e { @@ -566,7 +568,7 @@ impl BaseClient { state_events .entry(e.content().event_type().to_owned()) .or_insert_with(BTreeMap::new) - .insert(e.state_key().to_owned(), e); + .insert(e.state_key().to_owned(), raw_event.clone()); } } Err(err) => { @@ -595,19 +597,18 @@ impl BaseClient { let room_id = room_info.room_id.clone(); - for event in events - .iter() - .filter_map(|e| match hoist_and_deserialize_state_event(&e) { - Ok(e) => Some(e), - Err(err) => { + for raw_event in events { + let event = match hoist_and_deserialize_state_event(raw_event) { + Ok(e) => e, + Err(e) => { warn!( "Couldn't deserialize state event for room {}: {:?} {:#?}", - room_id, err, e + room_id, e, raw_event ); - None + continue; } - }) - { + }; + room_info.handle_state_event(&event.content()); if let AnySyncStateEvent::RoomMember(member) = event { @@ -641,7 +642,7 @@ impl BaseClient { state_events .entry(event.content().event_type().to_owned()) .or_insert_with(BTreeMap::new) - .insert(event.state_key().to_owned(), event); + .insert(event.state_key().to_owned(), raw_event.clone()); } } @@ -658,20 +659,24 @@ impl BaseClient { events: &[Raw], changes: &mut StateChanges, ) { - let events: Vec = - events.iter().filter_map(|e| e.deserialize().ok()).collect(); - - for event in &events { - changes.add_room_account_data(room_id, event.clone()); + for raw_event in events { + if let Ok(event) = raw_event.deserialize() { + changes.add_room_account_data(room_id, event, raw_event.clone()); + } } } async fn handle_account_data(&self, events: &[Raw], changes: &mut StateChanges) { - let events: Vec = - events.iter().filter_map(|e| e.deserialize().ok()).collect(); + let mut account_data = BTreeMap::new(); - for event in &events { - if let AnyBasicEvent::Direct(e) = event { + for raw_event in events { + let event = if let Ok(e) = raw_event.deserialize() { + e + } else { + continue; + }; + + if let AnyBasicEvent::Direct(e) = &event { for (user_id, rooms) in e.content.iter() { for room_id in rooms { if let Some(room) = changes.room_infos.get_mut(room_id) { @@ -684,12 +689,9 @@ impl BaseClient { } } } - } - let account_data: BTreeMap = events - .into_iter() - .map(|e| (e.content().event_type().to_owned(), e)) - .collect(); + account_data.insert(event.content().event_type().to_owned(), raw_event.clone()); + } changes.account_data = account_data; } @@ -904,7 +906,7 @@ impl BaseClient { .iter() .filter_map(|e| { let event = e.deserialize().ok()?; - Some((event.sender.clone(), event)) + Some((event.sender, e.clone())) }) .collect(); @@ -1345,14 +1347,17 @@ impl BaseClient { /// Gets the push rules from `changes` if they have been updated, otherwise get them from the /// store. As a fallback, uses `Ruleset::server_default` if the user is logged in. pub async fn get_push_rules(&self, changes: &StateChanges) -> Result { - if let Some(AnyBasicEvent::PushRules(event)) = - changes.account_data.get(&EventType::PushRules.to_string()) + if let Some(AnyBasicEvent::PushRules(event)) = changes + .account_data + .get(&EventType::PushRules.to_string()) + .and_then(|e| e.deserialize().ok()) { - Ok(event.content.global.clone()) + Ok(event.content.global) } else if let Some(AnyBasicEvent::PushRules(event)) = self .store .get_account_data_event(EventType::PushRules) .await? + .and_then(|e| e.deserialize().ok()) { Ok(event.content.global) } else if let Some(session) = self.get_session().await { @@ -1401,12 +1406,14 @@ impl BaseClient { .get(room_id) .and_then(|types| types.get(EventType::RoomPowerLevels.as_str())) .and_then(|events| events.get("")) + .and_then(|e| e.deserialize().ok()) { - event.content.clone() + event.content } else if let Some(AnySyncStateEvent::RoomPowerLevels(event)) = self .store .get_state_event(room_id, EventType::RoomPowerLevels, "") .await? + .and_then(|e| e.deserialize().ok()) { event.content } else { @@ -1454,8 +1461,9 @@ impl BaseClient { .get(room_id) .and_then(|types| types.get(EventType::RoomPowerLevels.as_str())) .and_then(|events| events.get("")) + .and_then(|e| e.deserialize().ok()) { - let room_power_levels = event.content.clone(); + let room_power_levels = event.content; push_rules.users_power_levels = room_power_levels.users; push_rules.default_power_level = room_power_levels.users_default; diff --git a/matrix_sdk_base/src/rooms/normal.rs b/matrix_sdk_base/src/rooms/normal.rs index 82e9429e..d9dc8862 100644 --- a/matrix_sdk_base/src/rooms/normal.rs +++ b/matrix_sdk_base/src/rooms/normal.rs @@ -393,7 +393,11 @@ impl Room { return Ok(None); }; - let presence = self.store.get_presence_event(user_id).await?; + let presence = self + .store + .get_presence_event(user_id) + .await? + .and_then(|e| e.deserialize().ok()); let profile = self.store.get_profile(self.room_id(), user_id).await?; let max_power_level = self.max_power_level(); let is_room_creator = self @@ -410,6 +414,7 @@ impl Room { .store .get_state_event(self.room_id(), EventType::RoomPowerLevels, "") .await? + .and_then(|e| e.deserialize().ok()) .and_then(|e| { if let AnySyncStateEvent::RoomPowerLevels(e) = e { Some(e) diff --git a/matrix_sdk_base/src/store/memory_store.rs b/matrix_sdk_base/src/store/memory_store.rs index 33ffd1a5..b8182cc1 100644 --- a/matrix_sdk_base/src/store/memory_store.rs +++ b/matrix_sdk_base/src/store/memory_store.rs @@ -23,10 +23,11 @@ use matrix_sdk_common::{ events::{ presence::PresenceEvent, room::member::{MemberEventContent, MembershipState}, - AnyBasicEvent, AnyStrippedStateEvent, AnySyncStateEvent, EventContent, EventType, + AnyBasicEvent, AnyStrippedStateEvent, AnySyncStateEvent, EventType, }, identifiers::{RoomId, UserId}, instant::Instant, + Raw, }; use tracing::info; @@ -39,7 +40,7 @@ use super::{Result, RoomInfo, StateChanges, StateStore}; pub struct MemoryStore { sync_token: Arc>>, filters: Arc>, - account_data: Arc>, + account_data: Arc>>, members: Arc>>, profiles: Arc>>, display_names: Arc>>>, @@ -47,14 +48,14 @@ pub struct MemoryStore { invited_user_ids: Arc>>, room_info: Arc>, #[allow(clippy::type_complexity)] - room_state: Arc>>>, - room_account_data: Arc>>, + room_state: Arc>>>>, + room_account_data: Arc>>>, stripped_room_info: Arc>, #[allow(clippy::type_complexity)] stripped_room_state: - Arc>>>, + Arc>>>>, stripped_members: Arc>>, - presence: Arc>, + presence: Arc>>, } impl MemoryStore { @@ -176,14 +177,14 @@ impl MemoryStore { } for (room, event_types) in &changes.state { - for events in event_types.values() { - for event in events.values() { + for (event_type, events) in event_types { + for (state_key, event) in events { self.room_state .entry(room.clone()) .or_insert_with(DashMap::new) - .entry(event.content().event_type().to_string()) + .entry(event_type.to_owned()) .or_insert_with(DashMap::new) - .insert(event.state_key().to_string(), event.clone()); + .insert(state_key.to_owned(), event.clone()); } } } @@ -211,14 +212,14 @@ impl MemoryStore { } for (room, event_types) in &changes.stripped_state { - for events in event_types.values() { - for event in events.values() { + for (event_type, events) in event_types { + for (state_key, event) in events { self.stripped_room_state .entry(room.clone()) .or_insert_with(DashMap::new) - .entry(event.content().event_type().to_string()) + .entry(event_type.to_owned()) .or_insert_with(DashMap::new) - .insert(event.state_key().to_string(), event.clone()); + .insert(state_key.to_owned(), event.clone()); } } } @@ -228,7 +229,7 @@ impl MemoryStore { Ok(()) } - async fn get_presence_event(&self, user_id: &UserId) -> Result> { + async fn get_presence_event(&self, user_id: &UserId) -> Result>> { #[allow(clippy::map_clone)] Ok(self.presence.get(user_id).map(|p| p.clone())) } @@ -238,7 +239,7 @@ impl MemoryStore { room_id: &RoomId, event_type: EventType, state_key: &str, - ) -> Result> { + ) -> Result>> { #[allow(clippy::map_clone)] Ok(self.room_state.get(room_id).and_then(|e| { e.get(event_type.as_ref()) @@ -304,7 +305,10 @@ impl MemoryStore { self.stripped_room_info.iter().map(|r| r.clone()).collect() } - async fn get_account_data_event(&self, event_type: EventType) -> Result> { + async fn get_account_data_event( + &self, + event_type: EventType, + ) -> Result>> { Ok(self .account_data .get(event_type.as_ref()) @@ -331,7 +335,7 @@ impl StateStore for MemoryStore { self.get_sync_token().await } - async fn get_presence_event(&self, user_id: &UserId) -> Result> { + async fn get_presence_event(&self, user_id: &UserId) -> Result>> { self.get_presence_event(user_id).await } @@ -340,7 +344,7 @@ impl StateStore for MemoryStore { room_id: &RoomId, event_type: EventType, state_key: &str, - ) -> Result> { + ) -> Result>> { self.get_state_event(room_id, event_type, state_key).await } @@ -393,7 +397,10 @@ impl StateStore for MemoryStore { .unwrap_or_default()) } - async fn get_account_data_event(&self, event_type: EventType) -> Result> { + async fn get_account_data_event( + &self, + event_type: EventType, + ) -> Result>> { self.get_account_data_event(event_type).await } } diff --git a/matrix_sdk_base/src/store/mod.rs b/matrix_sdk_base/src/store/mod.rs index d9e33336..2e017d22 100644 --- a/matrix_sdk_base/src/store/mod.rs +++ b/matrix_sdk_base/src/store/mod.rs @@ -31,7 +31,7 @@ use matrix_sdk_common::{ }, identifiers::{RoomId, UserId}, locks::RwLock, - AsyncTraitDeps, + AsyncTraitDeps, Raw, }; #[cfg(feature = "sled_state_store")] use sled::Db; @@ -114,7 +114,7 @@ pub trait StateStore: AsyncTraitDeps { /// /// * `user_id` - The id of the user for which we wish to fetch the presence /// event for. - async fn get_presence_event(&self, user_id: &UserId) -> Result>; + async fn get_presence_event(&self, user_id: &UserId) -> Result>>; /// Get a state event out of the state store. /// @@ -128,7 +128,7 @@ pub trait StateStore: AsyncTraitDeps { room_id: &RoomId, event_type: EventType, state_key: &str, - ) -> Result>; + ) -> Result>>; /// Get the current profile for the given user in the given room. /// @@ -192,7 +192,10 @@ pub trait StateStore: AsyncTraitDeps { /// # Arguments /// /// * `event_type` - The event type of the account data event. - async fn get_account_data_event(&self, event_type: EventType) -> Result>; + async fn get_account_data_event( + &self, + event_type: EventType, + ) -> Result>>; } /// A state store wrapper for the SDK. @@ -345,30 +348,33 @@ pub struct StateChanges { /// A user session, containing an access token and information about the associated user account. pub session: Option, /// A mapping of event type string to `AnyBasicEvent`. - pub account_data: BTreeMap, + pub account_data: BTreeMap>, /// A mapping of `UserId` to `PresenceEvent`. - pub presence: BTreeMap, + pub presence: BTreeMap>, /// A mapping of `RoomId` to a map of users and their `MemberEvent`. pub members: BTreeMap>, /// A mapping of `RoomId` to a map of users and their `MemberEventContent`. pub profiles: BTreeMap>, - pub(crate) ambiguity_maps: BTreeMap>>, /// A mapping of `RoomId` to a map of event type string to a state key and `AnySyncStateEvent`. - pub state: BTreeMap>>, + pub state: BTreeMap>>>, /// A mapping of `RoomId` to a map of event type string to `AnyBasicEvent`. - pub room_account_data: BTreeMap>, + pub room_account_data: BTreeMap>>, /// A map of `RoomId` to `RoomInfo`. pub room_infos: BTreeMap, /// A mapping of `RoomId` to a map of event type to a map of state key to `AnyStrippedStateEvent`. - pub stripped_state: BTreeMap>>, + pub stripped_state: + BTreeMap>>>, /// A mapping of `RoomId` to a map of users and their `StrippedMemberEvent`. pub stripped_members: BTreeMap>, /// A map of `RoomId` to `RoomInfo`. pub invited_room_info: BTreeMap, + /// A map from room id to a map of a display name and a set of user ids that + /// share that display name in the given room. + pub ambiguity_maps: BTreeMap>>, /// A map of `RoomId` to a vector of `Notification`s pub notifications: BTreeMap>, } @@ -383,8 +389,8 @@ impl StateChanges { } /// Update the `StateChanges` struct with the given `PresenceEvent`. - pub fn add_presence_event(&mut self, event: PresenceEvent) { - self.presence.insert(event.sender.clone(), event); + pub fn add_presence_event(&mut self, event: PresenceEvent, raw_event: Raw) { + self.presence.insert(event.sender, raw_event); } /// Update the `StateChanges` struct with the given `RoomInfo`. @@ -400,27 +406,22 @@ impl StateChanges { } /// Update the `StateChanges` struct with the given `AnyBasicEvent`. - pub fn add_account_data(&mut self, event: AnyBasicEvent) { + pub fn add_account_data(&mut self, event: AnyBasicEvent, raw_event: Raw) { self.account_data - .insert(event.content().event_type().to_owned(), event); + .insert(event.content().event_type().to_owned(), raw_event); } /// Update the `StateChanges` struct with the given room with a new `AnyBasicEvent`. - pub fn add_room_account_data(&mut self, room_id: &RoomId, event: AnyBasicEvent) { + pub fn add_room_account_data( + &mut self, + room_id: &RoomId, + event: AnyBasicEvent, + raw_event: Raw, + ) { self.room_account_data .entry(room_id.to_owned()) .or_insert_with(BTreeMap::new) - .insert(event.content().event_type().to_owned(), event); - } - - /// Update the `StateChanges` struct with the given room with a new `AnyStrippedStateEvent`. - pub fn add_stripped_state_event(&mut self, room_id: &RoomId, event: AnyStrippedStateEvent) { - self.stripped_state - .entry(room_id.to_owned()) - .or_insert_with(BTreeMap::new) - .entry(event.content().event_type().to_string()) - .or_insert_with(BTreeMap::new) - .insert(event.state_key().to_string(), event); + .insert(event.content().event_type().to_owned(), raw_event); } /// Update the `StateChanges` struct with the given room with a new `StrippedMemberEvent`. @@ -434,13 +435,18 @@ impl StateChanges { } /// Update the `StateChanges` struct with the given room with a new `AnySyncStateEvent`. - pub fn add_state_event(&mut self, room_id: &RoomId, event: AnySyncStateEvent) { + pub fn add_state_event( + &mut self, + room_id: &RoomId, + event: AnySyncStateEvent, + raw_event: Raw, + ) { self.state .entry(room_id.to_owned()) .or_insert_with(BTreeMap::new) .entry(event.content().event_type().to_string()) .or_insert_with(BTreeMap::new) - .insert(event.state_key().to_string(), event); + .insert(event.state_key().to_string(), raw_event); } /// Update the `StateChanges` struct with the given room with a new `Notification`. diff --git a/matrix_sdk_base/src/store/sled_store/mod.rs b/matrix_sdk_base/src/store/sled_store/mod.rs index ac815683..e14c0a0b 100644 --- a/matrix_sdk_base/src/store/sled_store/mod.rs +++ b/matrix_sdk_base/src/store/sled_store/mod.rs @@ -31,9 +31,10 @@ use matrix_sdk_common::{ events::{ presence::PresenceEvent, room::member::{MemberEventContent, MembershipState}, - AnyBasicEvent, AnySyncStateEvent, EventContent, EventType, + AnyBasicEvent, AnySyncStateEvent, EventType, }, identifiers::{RoomId, UserId}, + Raw, }; use serde::{Deserialize, Serialize}; @@ -405,14 +406,10 @@ impl SledStore { } for (room, event_types) in &changes.state { - for events in event_types.values() { - for event in events.values() { + for (event_type, events) in event_types { + for (state_key, event) in events { state.insert( - ( - room.as_str(), - event.content().event_type(), - event.state_key(), - ) + (room.as_str(), event_type.as_str(), state_key.as_str()) .encode(), self.serialize_event(&event) .map_err(ConflictableTransactionError::Abort)?, @@ -456,14 +453,10 @@ impl SledStore { } for (room, event_types) in &changes.stripped_state { - for events in event_types.values() { - for event in events.values() { + for (event_type, events) in event_types { + for (state_key, event) in events { stripped_state.insert( - ( - room.as_str(), - event.content().event_type(), - event.state_key(), - ) + (room.as_str(), event_type.as_str(), state_key.as_str()) .encode(), self.serialize_event(&event) .map_err(ConflictableTransactionError::Abort)?, @@ -485,7 +478,7 @@ impl SledStore { Ok(()) } - pub async fn get_presence_event(&self, user_id: &UserId) -> Result> { + pub async fn get_presence_event(&self, user_id: &UserId) -> Result>> { Ok(self .presence .get(user_id.encode())? @@ -498,7 +491,7 @@ impl SledStore { room_id: &RoomId, event_type: EventType, state_key: &str, - ) -> Result> { + ) -> Result>> { Ok(self .room_state .get((room_id.as_str(), event_type.as_str(), state_key).encode())? @@ -597,7 +590,7 @@ impl SledStore { pub async fn get_account_data_event( &self, event_type: EventType, - ) -> Result> { + ) -> Result>> { Ok(self .account_data .get(event_type.encode())? @@ -624,7 +617,7 @@ impl StateStore for SledStore { self.get_sync_token().await } - async fn get_presence_event(&self, user_id: &UserId) -> Result> { + async fn get_presence_event(&self, user_id: &UserId) -> Result>> { self.get_presence_event(user_id).await } @@ -633,7 +626,7 @@ impl StateStore for SledStore { room_id: &RoomId, event_type: EventType, state_key: &str, - ) -> Result> { + ) -> Result>> { self.get_state_event(room_id, event_type, state_key).await } @@ -682,7 +675,10 @@ impl StateStore for SledStore { .await } - async fn get_account_data_event(&self, event_type: EventType) -> Result> { + async fn get_account_data_event( + &self, + event_type: EventType, + ) -> Result>> { self.get_account_data_event(event_type).await } } @@ -693,12 +689,17 @@ mod test { use matrix_sdk_common::{ events::{ - room::member::{MemberEventContent, MembershipState}, - Unsigned, + room::{ + member::{MemberEventContent, MembershipState}, + power_levels::PowerLevelsEventContent, + }, + AnySyncStateEvent, EventType, Unsigned, }, identifiers::{room_id, user_id, EventId, UserId}, + Raw, }; use matrix_sdk_test::async_test; + use serde_json::json; use super::{SledStore, StateChanges}; use crate::deserialized_responses::MemberEvent; @@ -707,6 +708,22 @@ mod test { user_id!("@example:localhost") } + fn power_level_event() -> Raw { + let content = PowerLevelsEventContent::default(); + + let event = json!({ + "event_id": EventId::try_from("$h29iv0s8:example.com").unwrap(), + "content": content, + "sender": user_id(), + "type": "m.room.power_levels", + "origin_server_ts": 0u64, + "state_key": "", + "unsigned": Unsigned::default(), + }); + + serde_json::from_value(event).unwrap() + } + fn membership_event() -> MemberEvent { let content = MemberEventContent { avatar_url: None, @@ -752,4 +769,28 @@ mod test { .unwrap() .is_some()); } + + #[async_test] + async fn test_power_level_saving() { + let store = SledStore::open().unwrap(); + let room_id = room_id!("!test:localhost"); + + let raw_event = power_level_event(); + let event = raw_event.deserialize().unwrap(); + + assert!(store + .get_state_event(&room_id, EventType::RoomPowerLevels, "") + .await + .unwrap() + .is_none()); + let mut changes = StateChanges::default(); + changes.add_state_event(&room_id, event, raw_event); + + store.save_changes(&changes).await.unwrap(); + assert!(store + .get_state_event(&room_id, EventType::RoomPowerLevels, "") + .await + .unwrap() + .is_some()); + } }