From eedf4e72d1d1c122a4be8ef38c59e4af81dd0f5a Mon Sep 17 00:00:00 2001 From: Devin R Date: Thu, 21 May 2020 10:16:04 -0400 Subject: [PATCH 1/4] base_client: if unsigned field contains prev_content pull out and add to MemberEvent --- matrix_sdk_base/src/client.rs | 164 ++++++++++++++++++++++- matrix_sdk_base/src/event_emitter/mod.rs | 17 ++- test_data/events/member.json | 7 +- 3 files changed, 179 insertions(+), 9 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 45b42959..b4156eed 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -31,6 +31,7 @@ use crate::events::presence::PresenceEvent; use crate::events::collections::only::Event as NonRoomEvent; use crate::events::ignored_user_list::IgnoredUserListEvent; use crate::events::push_rules::{PushRulesEvent, Ruleset}; +use crate::events::room::member::MemberEventContent; use crate::events::stripped::AnyStrippedStateEvent; use crate::events::EventJson; use crate::identifiers::{RoomId, UserId}; @@ -60,6 +61,38 @@ use matrix_sdk_crypto::{OlmMachine, OneTimeKeys}; pub type Token = String; +/// A deserialization wrapper for extracting the prev_content field when +/// found in an `unsigned` field. +/// +/// Represents the outer `unsigned` field +#[derive(serde::Deserialize)] +pub struct AdditionalEventData { + unsigned: AdditionalUnsignedData, +} + +/// A deserialization wrapper for extracting the prev_content field when +/// found in an `unsigned` field. +/// +/// Represents the inner `prev_content` field +#[derive(serde::Deserialize)] +pub struct AdditionalUnsignedData { + pub prev_content: Option>, +} + +fn room_event_deserializer(event: &EventJson) -> Option { + serde_json::from_str::(event.json().get()) + .map(|more_unsigned| more_unsigned.unsigned) + .ok() +} + +fn stripped_event_deserializer( + event: &EventJson, +) -> Option { + serde_json::from_str::(event.json().get()) + .map(|more_unsigned| more_unsigned.unsigned) + .ok() +} + /// Signals to the `BaseClient` which `RoomState` to send to `EventEmitter`. #[derive(Debug)] pub enum RoomStateType { @@ -782,7 +815,17 @@ impl BaseClient { *event = e; } - if let Ok(e) = event.deserialize() { + if let Ok(mut e) = event.deserialize() { + // if the event is a m.room.member event the server will sometimes + // send the `prev_content` field as part of the unsigned field. + if let RoomEvent::RoomMember(member) = &mut e { + if let Some(raw_content) = room_event_deserializer(event) { + member.prev_content = match raw_content.prev_content { + Some(json) => json.deserialize().ok(), + None => None, + }; + } + } self.emit_timeline_event(&room_id, &e, RoomStateType::Joined) .await; } @@ -873,7 +916,17 @@ impl BaseClient { updated = true; }; - if let Ok(e) = event.deserialize() { + if let Ok(mut e) = event.deserialize() { + // if the event is a m.room.member event the server will sometimes + // send the `prev_content` field as part of the unsigned field. + if let RoomEvent::RoomMember(member) = &mut e { + if let Some(raw_content) = room_event_deserializer(event) { + member.prev_content = match raw_content.prev_content { + Some(json) => json.deserialize().ok(), + None => None, + }; + } + } self.emit_timeline_event(&room_id, &e, RoomStateType::Left) .await; } @@ -909,8 +962,26 @@ impl BaseClient { }; for event in &invited_room.invite_state.events { - if let Ok(e) = event.deserialize() { - self.emit_stripped_state_event(&room_id, &e, RoomStateType::Invited) + if let Ok(mut e) = event.deserialize() { + // if the event is a m.room.member event the server will sometimes + // send the `prev_content` field as part of the unsigned field. + if let AnyStrippedStateEvent::RoomMember(_) = &mut e { + if let Some(raw_content) = stripped_event_deserializer(event) { + let prev_content = match raw_content.prev_content { + Some(json) => json.deserialize().ok(), + None => None, + }; + self.emit_stripped_state_event( + &room_id, + &e, + prev_content, + RoomStateType::Invited, + ) + .await; + continue; + } + } + self.emit_stripped_state_event(&room_id, &e, None, RoomStateType::Invited) .await; } } @@ -1254,6 +1325,7 @@ impl BaseClient { &self, room_id: &RoomId, event: &AnyStrippedStateEvent, + prev_content: Option, room_state: RoomStateType, ) { let lock = self.event_emitter.read().await; @@ -1289,7 +1361,9 @@ impl BaseClient { match event { AnyStrippedStateEvent::RoomMember(member) => { - event_emitter.on_stripped_state_member(room, &member).await + event_emitter + .on_stripped_state_member(room, &member, prev_content) + .await } AnyStrippedStateEvent::RoomName(name) => { event_emitter.on_stripped_state_name(room, &name).await @@ -1626,6 +1700,86 @@ mod test { assert!(room.is_some()); } + #[async_test] + async fn test_prev_content_from_unsigned() { + use super::*; + + use crate::{EventEmitter, SyncRoom}; + use matrix_sdk_common::events::{ + room::member::{MemberEvent, MembershipChange}, + EventJson, + }; + use matrix_sdk_common::locks::RwLock; + use std::sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }; + + struct EE(Arc); + #[async_trait::async_trait] + impl EventEmitter for EE { + async fn on_room_member(&self, room: SyncRoom, event: &MemberEvent) { + if let SyncRoom::Joined(_) = room { + println!("joined"); + match event.membership_change() { + MembershipChange::Joined => { + self.0.swap(true, Ordering::SeqCst); + } + _ => {} + }; + } + if event.prev_content.is_none() { + println!("NOT found"); + self.0.swap(false, Ordering::SeqCst); + } + } + } + + let room_id = get_room_id(); + let passed = Arc::new(AtomicBool::default()); + let emitter = EE(Arc::clone(&passed)); + let mut client = get_client(); + + // fake a room event with no EventEmitter to build the correct Room + let mut sync_response = EventBuilder::default() + .add_room_event(EventsFile::Member, RoomEvent::RoomMember) + .build_sync_response(); + client + .receive_sync_response(&mut sync_response) + .await + .unwrap(); + + client.event_emitter = Arc::new(RwLock::new(Some(Box::new(emitter)))); + + let event = serde_json::from_str::>(include_str!( + "../../test_data/events/member.json" + )) + .unwrap(); + + // FIXME since the `SyncResponse` created using EventBuilder doesn't pull out the `prev_content` + // from the `unsigned` field we must do this manually. + // + // this is exactly what happens in `receive_sync_response` + if let Ok(mut e) = event.deserialize() { + // if the event is a m.room.member event the server will sometimes + // send the `prev_content` field as part of the unsigned field. + if let RoomEvent::RoomMember(member) = &mut e { + if let Some(raw_content) = room_event_deserializer(&event) { + member.prev_content = match raw_content.prev_content { + Some(json) => json.deserialize().ok(), + None => None, + }; + } + println!("{:?}", member.prev_content); + } + client + .emit_timeline_event(&room_id, &e, RoomStateType::Joined) + .await; + } + + assert!(passed.load(Ordering::SeqCst)) + } + #[async_test] #[cfg(feature = "encryption")] async fn test_group_session_invalidation() { diff --git a/matrix_sdk_base/src/event_emitter/mod.rs b/matrix_sdk_base/src/event_emitter/mod.rs index 0c72a7e6..f6a639c1 100644 --- a/matrix_sdk_base/src/event_emitter/mod.rs +++ b/matrix_sdk_base/src/event_emitter/mod.rs @@ -27,7 +27,7 @@ use crate::events::{ avatar::AvatarEvent, canonical_alias::CanonicalAliasEvent, join_rules::JoinRulesEvent, - member::MemberEvent, + member::{MemberEvent, MemberEventContent}, message::{feedback::FeedbackEvent, MessageEvent}, name::NameEvent, power_levels::PowerLevelsEvent, @@ -131,7 +131,13 @@ pub trait EventEmitter: Send + Sync { // `AnyStrippedStateEvent`s /// Fires when `Client` receives a `AnyStrippedStateEvent::StrippedRoomMember` event. - async fn on_stripped_state_member(&self, _: SyncRoom, _: &StrippedRoomMember) {} + async fn on_stripped_state_member( + &self, + _: SyncRoom, + _: &StrippedRoomMember, + _: Option, + ) { + } /// Fires when `Client` receives a `AnyStrippedStateEvent::StrippedRoomName` event. async fn on_stripped_state_name(&self, _: SyncRoom, _: &StrippedRoomName) {} /// Fires when `Client` receives a `AnyStrippedStateEvent::StrippedRoomCanonicalAlias` event. @@ -235,7 +241,12 @@ mod test { self.0.lock().await.push("state rules".to_string()) } - async fn on_stripped_state_member(&self, _: SyncRoom, _: &StrippedRoomMember) { + async fn on_stripped_state_member( + &self, + _: SyncRoom, + _: &StrippedRoomMember, + _: Option, + ) { self.0 .lock() .await diff --git a/test_data/events/member.json b/test_data/events/member.json index a1088ca9..10424dcb 100644 --- a/test_data/events/member.json +++ b/test_data/events/member.json @@ -12,6 +12,11 @@ "type": "m.room.member", "unsigned": { "age": 2970366338, - "replaces_state": "$151800111315tsynI:localhost" + "replaces_state": "$151800111315tsynI:localhost", + "prev_content": { + "avatar_url": null, + "displayname": "example", + "membership": "invite" + } } } From 20de6f6aea6db55418a39ab7fedee52a4febeb69 Mon Sep 17 00:00:00 2001 From: Devin R Date: Thu, 21 May 2020 10:27:29 -0400 Subject: [PATCH 2/4] base_client: prev_content deserialization TODO for if/when to mutate the event --- matrix_sdk_base/src/models/room.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index 554c5b90..093975e1 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -380,6 +380,8 @@ impl Room { /// /// Returns true if the joined member list changed, false otherwise. pub fn handle_membership(&mut self, event: &MemberEvent) -> bool { + // TODO this would not be handled correctly as all the MemberEvents have the `prev_content` + // inside of `unsigned` field match event.membership_change() { MembershipChange::Invited | MembershipChange::Joined => self.add_member(event), _ => { From 2f6bebdd1ab15dc80b065dcae56d353c3041d286 Mon Sep 17 00:00:00 2001 From: Devin R Date: Fri, 22 May 2020 17:12:58 -0400 Subject: [PATCH 3/4] base_client: replace event if prev_content deserialized from unsigned --- matrix_sdk_base/src/client.rs | 153 +++++++++++++++++++++------------- 1 file changed, 93 insertions(+), 60 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index b4156eed..10ea6742 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -79,13 +79,32 @@ pub struct AdditionalUnsignedData { pub prev_content: Option>, } -fn room_event_deserializer(event: &EventJson) -> Option { - serde_json::from_str::(event.json().get()) +/// If a `prev_content` field is found inside of `unsigned` we move it up to the events `prev_content` field. +fn deserialize_prev_content(event: &EventJson) -> Option> { + let prev_content = serde_json::from_str::(event.json().get()) .map(|more_unsigned| more_unsigned.unsigned) + .map(|additional| additional.prev_content) .ok() + .flatten(); + + if let Ok(mut ev) = event.deserialize() { + if let Some(prev) = prev_content { + match &mut ev { + RoomEvent::RoomMember(ref mut member) => { + member.prev_content = prev.deserialize().ok(); + Some(EventJson::from(ev)) + } + _ => None, + } + } else { + None + } + } else { + None + } } -fn stripped_event_deserializer( +fn stripped_deserialize_prev_content( event: &EventJson, ) -> Option { serde_json::from_str::(event.json().get()) @@ -505,6 +524,12 @@ impl BaseClient { room_id: &RoomId, event: &mut EventJson, ) -> (Option>, bool) { + // if the event is a m.room.member event the server will sometimes + // send the `prev_content` field as part of the unsigned field this extracts and + // places it where everything else expects it. + if let Some(ev) = deserialize_prev_content(event) { + *event = ev; + } match event.deserialize() { #[allow(unused_mut)] Ok(mut e) => { @@ -528,8 +553,8 @@ impl BaseClient { let room_lock = self.get_or_create_joined_room(&room_id).await; let mut room = room_lock.write().await; - if let RoomEvent::RoomMember(event) = &e { - let changed = room.handle_membership(event); + if let RoomEvent::RoomMember(mem_event) = &mut e { + let changed = room.handle_membership(mem_event); // The memberlist of the room changed, invalidate the group session // of the room. @@ -815,17 +840,11 @@ impl BaseClient { *event = e; } - if let Ok(mut e) = event.deserialize() { - // if the event is a m.room.member event the server will sometimes - // send the `prev_content` field as part of the unsigned field. - if let RoomEvent::RoomMember(member) = &mut e { - if let Some(raw_content) = room_event_deserializer(event) { - member.prev_content = match raw_content.prev_content { - Some(json) => json.deserialize().ok(), - None => None, - }; - } - } + if let Some(e) = deserialize_prev_content(&event) { + *event = e; + } + + if let Ok(e) = event.deserialize() { self.emit_timeline_event(&room_id, &e, RoomStateType::Joined) .await; } @@ -912,21 +931,15 @@ impl BaseClient { } for event in &mut left_room.timeline.events { + if let Some(e) = deserialize_prev_content(&event) { + *event = e; + } + if self.receive_left_timeline_event(room_id, &event).await { updated = true; }; - if let Ok(mut e) = event.deserialize() { - // if the event is a m.room.member event the server will sometimes - // send the `prev_content` field as part of the unsigned field. - if let RoomEvent::RoomMember(member) = &mut e { - if let Some(raw_content) = room_event_deserializer(event) { - member.prev_content = match raw_content.prev_content { - Some(json) => json.deserialize().ok(), - None => None, - }; - } - } + if let Ok(e) = event.deserialize() { self.emit_timeline_event(&room_id, &e, RoomStateType::Left) .await; } @@ -966,7 +979,7 @@ impl BaseClient { // if the event is a m.room.member event the server will sometimes // send the `prev_content` field as part of the unsigned field. if let AnyStrippedStateEvent::RoomMember(_) = &mut e { - if let Some(raw_content) = stripped_event_deserializer(event) { + if let Some(raw_content) = stripped_deserialize_prev_content(event) { let prev_content = match raw_content.prev_content { Some(json) => json.deserialize().ok(), None => None, @@ -1720,7 +1733,6 @@ mod test { impl EventEmitter for EE { async fn on_room_member(&self, room: SyncRoom, event: &MemberEvent) { if let SyncRoom::Joined(_) = room { - println!("joined"); match event.membership_change() { MembershipChange::Joined => { self.0.swap(true, Ordering::SeqCst); @@ -1729,7 +1741,6 @@ mod test { }; } if event.prev_content.is_none() { - println!("NOT found"); self.0.swap(false, Ordering::SeqCst); } } @@ -1740,42 +1751,64 @@ mod test { let emitter = EE(Arc::clone(&passed)); let mut client = get_client(); - // fake a room event with no EventEmitter to build the correct Room - let mut sync_response = EventBuilder::default() - .add_room_event(EventsFile::Member, RoomEvent::RoomMember) - .build_sync_response(); - client - .receive_sync_response(&mut sync_response) - .await - .unwrap(); - client.event_emitter = Arc::new(RwLock::new(Some(Box::new(emitter)))); - let event = serde_json::from_str::>(include_str!( + // This is needed other wise the `EventBuilder` goes through a de/ser cycle and the `prev_content` is lost. + let event = serde_json::from_str::(include_str!( "../../test_data/events/member.json" )) .unwrap(); - - // FIXME since the `SyncResponse` created using EventBuilder doesn't pull out the `prev_content` - // from the `unsigned` field we must do this manually. - // - // this is exactly what happens in `receive_sync_response` - if let Ok(mut e) = event.deserialize() { - // if the event is a m.room.member event the server will sometimes - // send the `prev_content` field as part of the unsigned field. - if let RoomEvent::RoomMember(member) = &mut e { - if let Some(raw_content) = room_event_deserializer(&event) { - member.prev_content = match raw_content.prev_content { - Some(json) => json.deserialize().ok(), - None => None, - }; - } - println!("{:?}", member.prev_content); + let mut joined_rooms: HashMap = HashMap::new(); + let joined_room = serde_json::json!({ + "summary": {}, + "account_data": { + "events": [], + }, + "ephemeral": { + "events": [], + }, + "state": { + "events": [], + }, + "timeline": { + "events": vec![ event ], + "limited": true, + "prev_batch": "t392-516_47314_0_7_1_1_1_11444_1" + }, + "unread_notifications": { + "highlight_count": 0, + "notification_count": 11 } - client - .emit_timeline_event(&room_id, &e, RoomStateType::Joined) - .await; - } + }); + joined_rooms.insert(room_id, joined_room); + + let empty_room: HashMap = HashMap::new(); + let body = serde_json::json!({ + "device_one_time_keys_count": {}, + "next_batch": "s526_47314_0_7_1_1_1_11444_1", + "device_lists": { + "changed": [], + "left": [] + }, + "rooms": { + "invite": empty_room, + "join": joined_rooms, + "leave": empty_room, + }, + "to_device": { + "events": [] + }, + "presence": { + "events": [] + } + }); + let response = http::Response::builder() + .body(serde_json::to_vec(&body).unwrap()) + .unwrap(); + let mut sync = + matrix_sdk_common::api::r0::sync::sync_events::Response::try_from(response).unwrap(); + + client.receive_sync_response(&mut sync).await.unwrap(); assert!(passed.load(Ordering::SeqCst)) } From ac4698f0d660aad5c0ca02eef6011551da3ba278 Mon Sep 17 00:00:00 2001 From: Devin R Date: Fri, 22 May 2020 21:29:51 -0400 Subject: [PATCH 4/4] base_client: only swap prev_content if outer is None --- matrix_sdk_base/src/client.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index dfe104ca..50107e81 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -85,22 +85,15 @@ fn deserialize_prev_content(event: &EventJson) -> Option { - member.prev_content = prev.deserialize().ok(); - Some(EventJson::from(ev)) - } - _ => None, - } - } else { - None + let mut ev = event.deserialize().ok()?; + match &mut ev { + RoomEvent::RoomMember(ref mut member) if member.prev_content.is_none() => { + member.prev_content = prev_content.deserialize().ok(); + Some(EventJson::from(ev)) } - } else { - None + _ => None, } }