Resolve some issues

STATUS.md

@@ -296,7 +296,7 @@ Legend:
     <th align="left" colspan="3">Presence</th>
   </tr>
   <tr>
-    <td align="center">:white_check_mark:</td>
+    <td align="center">:construction:</td>
     <td><a href="https://github.com/ruma/ruma/issues/39">#39</a></td>
     <td>PUT /presence/:user_id/status</td>
   </tr>
@@ -306,7 +306,7 @@ Legend:
     <td>GET /presence/:user_id/status</td>
   </tr>
   <tr>
-    <td align="center">:white_check_mark:</td>
+    <td align="center">:construction:</td>
     <td><a href="https://github.com/ruma/ruma/issues/41">#41</a></td>
     <td>POST /presence/list/:user_id</td>
   </tr>

src/api/r0/presence.rs

@@ -1,5 +1,7 @@
 //! Endpoints for presence.
 
+use std::time::SystemTime;
+
 use bodyparser;
 use iron::status::Status;
 use iron::{Chain, Handler, IronResult, IronError, Plugin, Request, Response};
@@ -70,19 +72,45 @@ pub struct GetPresenceStatus;
 
 middleware_chain!(GetPresenceStatus, [UserIdParam, AccessTokenAuth]);
 
+#[derive(Clone, Debug, Deserialize, Serialize)]
+struct GetPresenceStatusResponse {
+    /// The state message for this user if one was set.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    status_msg: Option<String>,
+    /// Whether the user is currently active.
+    currently_active: bool,
+    /// The length of time in milliseconds since an action was performed by this user.
+    last_active_ago: u64,
+    /// This user's presence. One of: ["online", "offline", "unavailable"]
+    presence: PresenceState,
+}
+
 impl Handler for GetPresenceStatus {
     fn handle(&self, request: &mut Request) -> IronResult<Response> {
         let user_id = request.extensions.get::<UserIdParam>()
             .expect("UserIdParam should ensure a UserId").clone();
 
         let connection = DB::from_request(request)?;
-        let config = Config::from_request(request)?;
 
-        let response = PresenceStatus::find_by_uid_and_convert_as_response(
-            &connection,
-            user_id,
-            config.update_interval_presence
-        )?;
+        let status = PresenceStatus::find_by_uid(&connection, &user_id)?;
+        let status: PresenceStatus = match status {
+            Some(event) => event,
+            None => return Err(IronError::from(
+                ApiError::not_found("The given user_id does not correspond to an presence status".to_string())
+            )),
+        };
+
+        let presence_state: PresenceState = status.presence.parse()
+            .expect("Something wrong with the database!");
+        let now = SystemTime::now();
+        let last_active_ago = PresenceStatus::calculate_last_active_ago(status.updated_at, now)?;
+
+        let response = GetPresenceStatusResponse {
+            status_msg: status.status_msg,
+            currently_active: PresenceState::Online == presence_state,
+            last_active_ago: last_active_ago,
+            presence: presence_state,
+        };
 
         Ok(Response::with((Status::Ok, SerializableResponse(response))))
     }
@@ -147,13 +175,11 @@ impl Handler for GetPresenceList {
             .expect("UserIdParam should ensure a UserId").clone();
 
         let connection = DB::from_request(request)?;
-        let config = Config::from_request(request)?;
 
         let (_, events) = PresenceList::find_events_by_uid(
             &connection,
             &user_id,
-            None,
-            config.update_interval_presence
+            None
         )?;
 
         Ok(Response::with((Status::Ok, SerializableResponse(events))))
@@ -278,10 +304,16 @@ mod tests {
         println!("{:#?}", events);
         let mut events = events.into_iter();
         assert_eq!(events.len(), 2);
-        let event = events.next().unwrap();
-        assert_eq!(event.find_path(&["content", "user_id"]).unwrap().as_str().unwrap(), bob_id);
-        let event = events.next().unwrap();
-        assert_eq!(event.find_path(&["content", "user_id"]).unwrap().as_str().unwrap(), carl_id);
+
+        assert_eq!(
+            events.next().unwrap().find_path(&["content", "user_id"]).unwrap().as_str().unwrap(),
+            bob_id
+        );
+
+        assert_eq!(
+            events.next().unwrap().find_path(&["content", "user_id"]).unwrap().as_str().unwrap(),
+            carl_id
+        );
     }
 
     #[test]
@@ -298,6 +330,20 @@ mod tests {
         assert_eq!(response.status, Status::UnprocessableEntity);
     }
 
+    #[test]
+    fn to_dropped_does_not_exist_presence_list() {
+        let test = Test::new();
+        let access_token = test.create_access_token_with_username("alice");
+
+        let presence_list_path = format!(
+            "/_matrix/client/r0/presence/list/{}?access_token={}",
+            "@alice:ruma.test",
+            access_token
+        );
+        let response = test.post(&presence_list_path, r#"{"invite":[], "drop": ["@carl:ruma.test"]}"#);
+        assert_eq!(response.status, Status::UnprocessableEntity);
+    }
+
     #[test]
     fn test_drop_presence_list() {
         let test = Test::new();

src/api/r0/sync.rs

@@ -85,7 +85,7 @@ impl Handler for Sync {
             timeout: timeout,
         };
 
-        let response = query::Sync::sync(&connection, &config.domain, &user, options, config.update_interval_presence)?;
+        let response = query::Sync::sync(&connection, &config.domain, &user, options)?;
 
         Ok(Response::with((Status::Ok, SerializableResponse(response))))
     }
@@ -424,10 +424,17 @@ mod tests {
             .unwrap();
         let mut events = array.into_iter();
         assert_eq!(events.len(), 2);
-        let event = events.next().unwrap();
-        assert_eq!(event.find_path(&["content", "user_id"]).unwrap().as_str().unwrap(), bob_id);
-        let event = events.next().unwrap();
-        assert_eq!(event.find_path(&["content", "user_id"]).unwrap().as_str().unwrap(), carl_id);
+
+        assert_eq!(
+            events.next().unwrap().find_path(&["content", "user_id"]).unwrap().as_str().unwrap(),
+            bob_id
+        );
+
+        assert_eq!(
+            events.next().unwrap().find_path(&["content", "user_id"]).unwrap().as_str().unwrap(),
+            carl_id
+        );
+
         let next_batch = Test::get_next_batch(&response);
         let options = SyncOptions {
             filter: None,

src/config.rs

@@ -25,13 +25,6 @@ struct RawConfig {
     domain: String,
     macaroon_secret_key: String,
     postgres_url: String,
-    #[serde(default = "default_update_interval_presence")]
-    update_interval_presence: u64,
-}
-
-/// Return the default value for update_interval_presence
-fn default_update_interval_presence() -> u64 {
-    300000
 }
 
 /// Server configuration provided by the user.
@@ -51,8 +44,6 @@ pub struct Config {
     /// A [PostgreSQL connection string](http://www.postgresql.org/docs/current/static/libpq-connect.html#LIBPQ-CONNSTRING)
     /// for Ruma's PostgreSQL database.
     pub postgres_url: String,
-    /// Interval after a user is set as unavailable in milliseconds. Defaults to 300000.
-    pub update_interval_presence: u64,
 }
 
 impl Config {
@@ -86,7 +77,6 @@ impl Config {
             domain: config.domain,
             macaroon_secret_key: macaroon_secret_key,
             postgres_url: config.postgres_url,
-            update_interval_presence: config.update_interval_presence,
         })
     }
 

src/models/presence_event.rs

@@ -65,7 +65,8 @@ impl PresenceStreamEvent {
             .map_err(ApiError::from)
     }
 
-    pub fn find_for_presence_list_by_uid(
+    /// Return `PresenceStreamEvent` for given `UserId` and `since`.
+    pub fn find_events_by_uid(
         connection: &PgConnection,
         user_id: &UserId,
         since: Option<i64>

src/models/presence_list.rs

@@ -43,11 +43,23 @@ impl PresenceList {
         drop: Vec<UserId>
     ) -> Result<(), ApiError> {
         connection.transaction::<(()), ApiError, _>(|| {
-
-            let invite = User::find_missing_user_and_check_existence(
+            let missing_user_ids = User::find_missing_users(
                 connection,
                 invite
             )?;
+            if !missing_user_ids.is_empty() {
+                return Err(
+                    ApiError::bad_json(format!(
+                        "Unknown users in invite list: {}",
+                        &missing_user_ids
+                            .iter()
+                            .map(|user_id| user_id.to_string())
+                            .collect::<Vec<String>>()
+                            .join(", ")
+                    ))
+                )
+            }
+
 
             let mut invites: Vec<PresenceList> = Vec::new();
             for observed_user in invite.clone() {
@@ -61,10 +73,22 @@ impl PresenceList {
                 .execute(connection)
                 .map_err(ApiError::from)?;
 
-            let drop = User::find_missing_user_and_check_existence(
+            let missing_user_ids = User::find_missing_users(
                 connection,
                 &drop
             )?;
+            if !missing_user_ids.is_empty() {
+                return Err(
+                    ApiError::bad_json(format!(
+                        "Unknown users in drop list: {}",
+                        &missing_user_ids
+                            .iter()
+                            .map(|user_id| user_id.to_string())
+                            .collect::<Vec<String>>()
+                            .join(", ")
+                    ))
+                )
+            }
 
             let drop = presence_list::table
                 .filter(presence_list::user_id.eq(user_id))
@@ -80,18 +104,17 @@ impl PresenceList {
     pub fn find_events_by_uid(
         connection: &PgConnection,
         user_id: &UserId,
-        since: Option<i64>,
-        timeout: u64
+        since: Option<i64>
     ) -> Result<(i64, Vec<PresenceEvent>), ApiError> {
         let mut max_ordering = -1;
 
-        let stream_events= PresenceStreamEvent::find_for_presence_list_by_uid(
+        let stream_events= PresenceStreamEvent::find_events_by_uid(
             connection,
             user_id,
             since
         )?;
 
-        let profiles = Profile::find_for_presence_list_by_uid(
+        let profiles = Profile::find_profiles_by_presence_list(
             connection,
             user_id
         )?;
@@ -109,16 +132,13 @@ impl PresenceList {
                 displayname = profile.displayname.clone();
             }
 
-            let mut presence_state: PresenceState = stream_event.presence.parse().expect("Something wrong with the database!");
+            let presence_state: PresenceState = stream_event.presence.parse().expect("Something wrong with the database!");
             let last_active_ago = PresenceStatus::calculate_last_active_ago(stream_event.created_at, now)?;
-            if last_active_ago > timeout && presence_state == PresenceState::Online {
-                presence_state = PresenceState::Unavailable;
-            }
 
             events.push(PresenceEvent {
                 content: PresenceEventContent {
                     avatar_url: avatar_url,
-                    currently_active: true,
+                    currently_active: PresenceState::Online == presence_state,
                     displayname: displayname,
                     last_active_ago: Some(last_active_ago),
                     presence: presence_state,

src/models/presence_status.rs

@@ -22,19 +22,6 @@ use error::ApiError;
 use models::presence_event::PresenceStreamEvent;
 use schema::presence_status;
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
-pub struct GetPresenceStatusResponse {
-    /// The state message for this user if one was set.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    status_msg: Option<String>,
-    /// Whether the user is currently active.
-    currently_active: bool,
-    /// The length of time in milliseconds since an action was performed by this user.
-    last_active_ago: u64,
-    /// This user's presence. One of: ["online", "offline", "unavailable"]
-    presence: PresenceState,
-}
-
 /// A Matrix presence status, not saved yet.
 #[derive(Debug, Clone, Insertable)]
 #[table_name = "presence_status"]
@@ -142,34 +129,6 @@ impl PresenceStatus {
         }
     }
 
-    pub fn find_by_uid_and_convert_as_response(
-        connection: &PgConnection,
-        user_id: UserId,
-        timeout: u64
-    ) -> Result<GetPresenceStatusResponse, ApiError> {
-        let status = PresenceStatus::find_by_uid(&connection, &user_id)?;
-        let status: PresenceStatus = match status {
-            Some(event) => event,
-            None => return Err(
-                ApiError::not_found("The given user_id does not correspond to an presence status".to_string())
-            ),
-        };
-
-        let mut presence_state: PresenceState = status.presence.parse().expect("Something wrong with the database!");
-        let now = SystemTime::now();
-        let last_active_ago = PresenceStatus::calculate_last_active_ago(status.updated_at, now)?;
-        if last_active_ago > timeout && presence_state == PresenceState::Online {
-            presence_state = PresenceState::Unavailable;
-        }
-
-        Ok(GetPresenceStatusResponse {
-            status_msg: status.status_msg,
-            currently_active: PresenceState::Online == presence_state,
-            last_active_ago: last_active_ago,
-            presence: presence_state,
-        })
-    }
-
     /// Calculate the difference between two SystemTimes in milliseconds.
     pub fn calculate_last_active_ago(since: SystemTime, now: SystemTime) -> Result<u64, ApiError> {
         let elapsed = now.duration_since(since).map_err(ApiError::from)?;

src/models/profile.rs

@@ -131,7 +131,8 @@ impl Profile {
         }
     }
 
-    pub fn find_for_presence_list_by_uid(
+    /// Return `Profile`s for given `UserId` and his `PresenceList` entries.
+    pub fn find_profiles_by_presence_list(
         connection: &PgConnection,
         user_id: &UserId,
     ) -> Result<Vec<Profile>, ApiError> {

src/models/room_membership.rs

@@ -318,10 +318,21 @@ impl RoomMembership {
         invite_list: &Vec<UserId>,
         homeserver_domain: &str
     ) -> Result<(), ApiError> {
+        let missing_user_ids = User::find_missing_users(connection, invite_list)?;
+        if !missing_user_ids.is_empty() {
+            return Err(
+                ApiError::bad_json(format!(
+                    "Unknown users in invite list: {}",
+                    &missing_user_ids
+                        .iter()
+                        .map(|user_id| user_id.to_string())
+                        .collect::<Vec<String>>()
+                        .join(", ")
+                ))
+            )
+        }
 
-        let user_ids = User::find_missing_user_and_check_existence(connection, invite_list)?;
-
-        let options = user_ids.iter().map(|user_id| {
+        let options = invite_list.iter().map(|user_id| {
             RoomMembershipOptions {
                 room_id: room.id.clone(),
                 user_id: user_id.clone(),