fix review issues

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

src/base_client.rs

@@ -205,11 +205,16 @@ impl Client {
             ))))
     }
 
+    pub(crate) fn get_room(&mut self, room_id: &str) -> Option<&mut Arc<RwLock<Room>>> {
+        #[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.
+        // this should be the room that was just created in the `Client::sync` loop.
-        let mut room = self.get_or_create_room(room_id).write().unwrap();
+        if let Some(room) = self.get_room(room_id) {
-        room.receive_presence_event(event)
+            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.

src/event_emitter/mod.rs

@@ -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<dyn EventEmitter>
-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) {}
-}

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;
 

src/models/room.rs

@@ -69,6 +69,7 @@ pub struct Room {
     pub members: HashMap<UserId, RoomMember>,
     /// A list of users that are currently typing.
     pub typing_users: Vec<UserId>,
+    // 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<UserId, RoomMember>) -> 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::<Vec<_>>();
 
             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

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,
         }