From 62159cc6dc8bdea5895358359b6d5c95c63e6a56 Mon Sep 17 00:00:00 2001 From: Devin R Date: Tue, 31 Mar 2020 06:57:29 -0400 Subject: [PATCH] fix review issues --- src/async_client.rs | 2 +- src/base_client.rs | 21 ++++++++++++------ src/event_emitter/mod.rs | 45 --------------------------------------- src/lib.rs | 1 - src/models/room.rs | 8 +++---- src/models/room_member.rs | 10 ++++++--- 6 files changed, 27 insertions(+), 60 deletions(-) delete mode 100644 src/event_emitter/mod.rs diff --git a/src/async_client.rs b/src/async_client.rs index 039a29ad..fa5c30cc 100644 --- a/src/async_client.rs +++ b/src/async_client.rs @@ -541,7 +541,7 @@ impl AsyncClient { } } - // TODO do we need `IncomingEphemeral` events? + // TODO `IncomingEphemeral` events for typing events // After the room has been created and state/timeline events accounted for we use the room_id of the newly created // room to add any presence events that relate to a user in the current room. This is not super diff --git a/src/base_client.rs b/src/base_client.rs index e6f15f2d..613c189c 100644 --- a/src/base_client.rs +++ b/src/base_client.rs @@ -205,11 +205,16 @@ impl Client { )))) } + pub(crate) fn get_room(&mut self, room_id: &str) -> Option<&mut Arc>> { + #[allow(clippy::or_fun_call)] + self.joined_rooms.get_mut(room_id) + } + /// Handle a m.ignored_user_list event, updating the room state if necessary. /// /// Returns true if the room name changed, false otherwise. pub(crate) fn handle_ignored_users(&mut self, event: &IgnoredUserListEvent) -> bool { - // TODO use actual UserId instead of string? + // FIXME when UserId becomes more like a &str wrapper in ruma-identifiers if self.ignored_users == event .content @@ -306,7 +311,7 @@ impl Client { room.receive_state_event(event) } - /// Receive a presence event from an `IncomingResponse` and updates the client state. + /// Receive a presence event from a sync response and updates the client state. /// /// Returns true if the membership list of the room changed, false /// otherwise. @@ -325,12 +330,16 @@ impl Client { if self.current_room_id.comes_after(user_id, event) { self.current_room_id.update(room_id, event); } - // this should be guaranteed to find the room that was just created in the `Client::sync` loop. - let mut room = self.get_or_create_room(room_id).write().unwrap(); - room.receive_presence_event(event) + // this should be the room that was just created in the `Client::sync` loop. + if let Some(room) = self.get_room(room_id) { + let mut room = room.write().unwrap(); + room.receive_presence_event(event) + } else { + false + } } - /// Receive a presence event from an `IncomingResponse` and updates the client state. + /// Receive a presence event from a sync response and updates the client state. /// /// This will only update the user if found in the current room looped through by `AsyncClient::sync`. /// Returns true if the specific users presence has changed, false otherwise. diff --git a/src/event_emitter/mod.rs b/src/event_emitter/mod.rs deleted file mode 100644 index 287d59f3..00000000 --- a/src/event_emitter/mod.rs +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2020 Damir Jelić -// Copyright 2020 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use crate::events::collections::all::RoomEvent; -use crate::models::Room; - -// JUST AN IDEA -// - -/// This is just a thought I had. Making users impl a trait instead of writing callbacks for events -/// could give the chance for really good documentation for each event? -/// It would look something like this -/// -/// ```rust,ignore -/// use matrix-sdk::{AsyncClient, EventEmitter}; -/// -/// struct MyAppClient; -/// -/// impl EventEmitter for MyAppClient { -/// fn on_room_member(&mut self, room: &Room, event: &RoomEvent) { ... } -/// } -/// async fn main() { -/// let cl = AsyncClient::with_emitter(MyAppClient); -/// } -/// ``` -/// -/// And in `AsyncClient::sync` there could be a switch case that calls the corresponding method on -/// the `Box -pub trait EventEmitter { - fn on_room_name(&mut self, _: &Room, _: &RoomEvent) {} - /// Any event that alters the state of the room's members - fn on_room_member(&mut self, _: &Room, _: &RoomEvent) {} -} diff --git a/src/lib.rs b/src/lib.rs index 6014c21d..c8813589 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,7 +35,6 @@ pub use ruma_identifiers as identifiers; mod async_client; mod base_client; mod error; -mod event_emitter; mod models; mod session; diff --git a/src/models/room.rs b/src/models/room.rs index bdbea9a8..9e847295 100644 --- a/src/models/room.rs +++ b/src/models/room.rs @@ -69,6 +69,7 @@ pub struct Room { pub members: HashMap, /// A list of users that are currently typing. pub typing_users: Vec, + // TODO when encryption events are handled we store algorithm used and rotation time. /// A flag indicating if the room is encrypted. pub encrypted: bool, /// Number of unread notifications with highlight flag set. @@ -94,7 +95,7 @@ impl RoomName { } pub fn calculate_name(&self, room_id: &str, members: &HashMap) -> String { - // https://github.com/matrix-org/matrix-js-sdk/blob/33941eb37bffe41958ba9887fc8070dfb1a0ee76/src/models/room.js#L1823 + // https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room. // the order in which we check for a name ^^ if let Some(name) = &self.name { name.clone() @@ -103,7 +104,6 @@ impl RoomName { } else if !self.aliases.is_empty() { self.aliases[0].alias().to_string() } else { - // TODO let mut names = members .values() .flat_map(|m| m.user.display_name.clone()) @@ -111,7 +111,7 @@ impl RoomName { .collect::>(); if names.is_empty() { - // TODO implement the rest of matrix-js-sdk handling of room names + // TODO implement the rest of display name for room spec format!("Room {}", room_id) } else { // stabilize order @@ -180,7 +180,7 @@ impl Room { match event.membership_change() { MembershipChange::Invited | MembershipChange::Joined => self.add_member(event), _ => { - if let Some(member) = self.members.get_mut(&event.sender.to_string()) { + if let Some(member) = self.members.get_mut(&event.state_key) { member.update_member(event) } else { false diff --git a/src/models/room_member.rs b/src/models/room_member.rs index c5319a6c..d86eeff0 100644 --- a/src/models/room_member.rs +++ b/src/models/room_member.rs @@ -13,6 +13,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::convert::TryFrom; + use super::User; use crate::api::r0 as api; use crate::events::collections::all::{Event, RoomEvent, StateEvent}; @@ -36,8 +38,11 @@ use crate::crypto::{OlmMachine, OneTimeKeys}; #[cfg(feature = "encryption")] use ruma_client_api::r0::keys::{upload_keys::Response as KeysUploadResponse, DeviceKeys}; +// Notes: if Alice invites Bob into a room we will get an event with the sender as Alice and the state key as Bob. + #[derive(Debug)] /// A Matrix room member. +/// pub struct RoomMember { /// The unique mxid of the user. pub user_id: UserId, @@ -64,13 +69,12 @@ impl RoomMember { let user = User::new(event); Self { room_id: event.room_id.as_ref().map(|id| id.to_string()), - user_id: event.sender.clone(), + user_id: UserId::try_from(event.state_key.as_str()).unwrap(), typing: None, user, power_level: None, power_level_norm: None, membership: event.content.membership, - // TODO should this be `sender` ?? name: event.state_key.clone(), events: vec![Event::RoomMember(event.clone())], } @@ -92,7 +96,7 @@ impl RoomMember { } NotImplemented => false, None => false, - // TODO should this be handled somehow ?? + // we ignore the error here as only a buggy or malicious server would send this Error => false, _ => false, }