Address some review issues fmt, errors, comments

src/client_server/backup.rs

@@ -256,7 +256,7 @@ pub async fn get_backup_key_session_route(
     let key_data = db
         .key_backups
         .get_session(&sender_user, &body.version, &body.room_id, &body.session_id)?
-        .ok_or_else(|| Error::BadDatabase("Backup key not found for this user's session"))?;
+        .ok_or_else(|| Error::bad_database("Backup key not found for this user's session."))?;
 
     Ok(get_backup_key_session::Response { key_data }.into())
 }

src/client_server/media.rs

@@ -39,7 +39,7 @@ pub async fn create_content_route(
     db.media.create(
         mxc.clone(),
         &body.filename.as_deref(),
-        &body.content_type.as_deref(), // TODO this is now optional handle
+        &body.content_type.as_deref(),
         &body.file,
     )?;
 

src/client_server/membership.rs

@@ -519,7 +519,6 @@ async fn join_room_by_id_helper(
         canon_json_stub.remove("event_id");
 
         // In order to create a compatible ref hash (EventID) the `hashes` field needs to be present
-        // who the hell knew...
         ruma::signatures::hash_and_sign_event(
             db.globals.server_name().as_str(),
             db.globals.keypair(),
@@ -602,7 +601,6 @@ async fn join_room_by_id_helper(
             )))) // Add join event we just created
             .map(|r| {
                 let (event_id, value) = r?;
-                // TODO remove .clone when I remove debug logging
                 state_res::StateEvent::from_id_value(event_id.clone(), value.clone())
                     .map(|ev| (event_id, Arc::new(ev)))
                     .map_err(|e| {

src/database/rooms.rs

@@ -35,6 +35,11 @@ use super::admin::AdminCommand;
 /// hashing the entire state.
 pub type StateHashId = IVec;
 
+/// An enum that represents the two valid states when searching
+/// for an events "parent".
+///
+/// An events parent is any event we are aware of that is part of
+/// the events `prev_events` array.
 pub enum ClosestParent {
     Append,
     Insert(u64),
@@ -80,7 +85,7 @@ impl StateStore for Rooms {
             .map_err(StateError::custom)?
             .ok_or_else(|| {
                 StateError::NotFound(format!(
-                    "PDU via room_id and event_id not found in the db.\n{}",
+                    "PDU via room_id and event_id not found in the db: {}",
                     event_id.as_str()
                 ))
             })?;
@@ -258,6 +263,8 @@ impl Rooms {
     }
 
     /// Force the creation of a new StateHash and insert it into the db.
+    ///
+    /// Whatever `state` is supplied to `force_state` __is__ the current room state snapshot.
     pub fn force_state(
         &self,
         room_id: &RoomId,
@@ -403,6 +410,12 @@ impl Rooms {
         }
     }
 
+    /// Recursively search for a PDU from our DB that is also in the
+    /// `prev_events` field of the incoming PDU.
+    ///
+    /// First we check if the last PDU inserted to the given room is a parent
+    /// if not we recursively check older `prev_events` to insert the incoming
+    /// event after.
     pub fn get_closest_parent(
         &self,
         room: &RoomId,

src/database/sending.rs

@@ -2,7 +2,7 @@ use std::{collections::HashMap, convert::TryFrom, time::SystemTime};
 
 use crate::{server_server, utils, Error, PduEvent, Result};
 use federation::transactions::send_transaction_message;
-use log::debug;
+use log::{debug, error};
 use rocket::futures::stream::{FuturesUnordered, StreamExt};
 use ruma::{api::federation, ServerName};
 use sled::IVec;
@@ -115,8 +115,8 @@ impl Sending {
                                     // servercurrentpdus with the prefix should be empty now
                                 }
                             }
-                            Err((_server, _e)) => {
+                            Err((server, e)) => {
-                                log::error!("server: {}\nerror: {}", _server, _e)
+                                error!("server: {}\nerror: {}", server, e)
                                 // TODO: exponential backoff
                             }
                         };

src/ruma_wrapper.rs

@@ -67,27 +67,28 @@ where
 
             let (sender_user, sender_device) =
             // TODO: Do we need to matches! anything else here? ServerSignatures 
-                if matches!(T::METADATA.authentication, AuthScheme::AccessToken | AuthScheme::QueryOnlyAccessToken) {
+                match T::METADATA.authentication {
-                    // Get token from header or query value
+                    AuthScheme::AccessToken | AuthScheme::QueryOnlyAccessToken => {
-                    let token = match request
+                        // Get token from header or query value
-                        .headers()
+                        let token = match request
-                        .get_one("Authorization")
+                            .headers()
-                        .map(|s| s[7..].to_owned()) // Split off "Bearer "
+                            .get_one("Authorization")
-                        .or_else(|| request.get_query_value("access_token").and_then(|r| r.ok()))
+                            .map(|s| s[7..].to_owned()) // Split off "Bearer "
-                    {
+                            .or_else(|| request.get_query_value("access_token").and_then(|r| r.ok()))
-                        // TODO: M_MISSING_TOKEN
+                        {
-                        None => return Failure((Status::Unauthorized, ())),
+                            // TODO: M_MISSING_TOKEN
-                        Some(token) => token,
+                            None => return Failure((Status::Unauthorized, ())),
-                    };
+                            Some(token) => token,
-
+                        };
-                    // Check if token is valid
+    
-                    match db.users.find_from_token(&token).unwrap() {
+                        // Check if token is valid
-                        // TODO: M_UNKNOWN_TOKEN
+                        match db.users.find_from_token(&token).unwrap() {
-                        None => return Failure((Status::Unauthorized, ())),
+                            // TODO: M_UNKNOWN_TOKEN
-                        Some((user_id, device_id)) => (Some(user_id), Some(device_id.into())),
+                            None => return Failure((Status::Unauthorized, ())),
+                            Some((user_id, device_id)) => (Some(user_id), Some(device_id.into())),
+                        }
                     }
-                } else {
+                    _ => (None, None)
-                    (None, None)
                 };
 
             let mut http_request = http::Request::builder()

src/server_server.rs

@@ -4,7 +4,7 @@ use crate::{
 };
 use get_profile_information::v1::ProfileField;
 use http::header::{HeaderValue, AUTHORIZATION, HOST};
-use log::warn;
+use log::{error, warn};
 use rocket::{get, post, put, response::content::Json, State};
 use ruma::{
     api::{
@@ -26,7 +26,6 @@ use std::{
     collections::BTreeMap,
     convert::{TryFrom, TryInto},
     fmt::Debug,
-    sync::Arc,
     time::{Duration, SystemTime},
 };
 use trust_dns_resolver::AsyncResolver;
@@ -99,7 +98,7 @@ where
     let mut http_request = request
         .try_into_http_request(&actual_destination, Some(""))
         .map_err(|e| {
-            warn!("failed to find destination {}: {}", actual_destination, e);
+            warn!("Failed to find destination {}: {}", actual_destination, e);
             Error::BadServerResponse("Invalid destination")
         })?;
 
@@ -264,12 +263,14 @@ pub fn get_server_keys(db: State<'_, Database>) -> Json<String> {
         .body(),
     )
     .unwrap();
+
     ruma::signatures::sign_json(
         db.globals.server_name().as_str(),
         db.globals.keypair(),
         &mut response,
     )
     .unwrap();
+
     Json(ruma::serde::to_canonical_json_string(&response).expect("JSON is canonical"))
 }
 
@@ -413,8 +414,8 @@ pub async fn send_transaction_message_route<'a>(
                 "m.receipt" => {}
                 _ => {}
             },
-            Err(_err) => {
+            Err(err) => {
-                log::error!("{}", _err);
+                error!("{}", err);
                 continue;
             }
         }
@@ -434,11 +435,9 @@ pub async fn send_transaction_message_route<'a>(
             .expect("all ruma pdus are conduit pdus");
         let room_id = &pdu.room_id;
 
-        // If we have no idea about this room
+        // If we have no idea about this room skip the PDU
-        // TODO: Does a server only send us events that we should know about or
-        // when everyone on this server leaves a room can we ignore those events?
         if !db.rooms.exists(&pdu.room_id)? {
-            log::error!("Room does not exist on this server");
+            error!("Room does not exist on this server.");
             resolved_map.insert(event_id, Err("Room is unknown to this server".into()));
             continue;
         }
@@ -460,7 +459,7 @@ pub async fn send_transaction_message_route<'a>(
             // As an example a possible error
             // {"errcode":"M_FORBIDDEN","error":"Host not in room."}
             Err(err) => {
-                log::error!("Request failed: {}", err);
+                error!("Request failed: {}", err);
                 resolved_map.insert(event_id, Err(err.to_string()));
                 continue;
             }
@@ -487,7 +486,7 @@ pub async fn send_transaction_message_route<'a>(
 
         if value.get("state_key").is_none() {
             if !db.rooms.is_joined(&pdu.sender, &pdu.room_id)? {
-                log::error!("Sender is not joined {}", pdu.kind);
+                error!("Sender is not joined {}", pdu.kind);
                 resolved_map.insert(event_id, Err("User is not in this room".into()));
                 continue;
             }