From 010c5b4705b117dea20fd1ec54093901749685b0 Mon Sep 17 00:00:00 2001 From: farodin91 Date: Tue, 17 Jan 2017 22:19:05 +0100 Subject: [PATCH] Fixing some timing issues and resolve some typos --- Cargo.lock | 1 - Cargo.toml | 1 - src/api/r0/presence.rs | 66 +++++++++++++++++++++++++++++------ src/api/r0/profile.rs | 11 ------ src/main.rs | 1 - src/models/presence_list.rs | 15 ++++---- src/models/presence_status.rs | 28 +++++++++++---- src/models/room_membership.rs | 6 ++-- src/query.rs | 3 +- 9 files changed, 88 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66fc8229..2d152fca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -28,7 +28,6 @@ dependencies = [ "serde_derive 0.8.21 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 0.8.4 (registry+https://github.com/rust-lang/crates.io-index)", "serde_yaml 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", - "time 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)", "toml 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "unicase 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index af057e28..8c468a99 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,6 @@ serde = "0.8.21" serde_derive = "0.8.21" serde_json = "0.8.4" serde_yaml = "0.5.0" -time = "0.1" toml = "0.2.1" unicase = "1.4.0" url = "1.2.4" diff --git a/src/api/r0/presence.rs b/src/api/r0/presence.rs index bdcb64b6..99839ca5 100644 --- a/src/api/r0/presence.rs +++ b/src/api/r0/presence.rs @@ -5,7 +5,6 @@ use iron::status::Status; use iron::{Chain, Handler, IronResult, IronError, Plugin, Request, Response}; use ruma_identifiers::UserId; use ruma_events::presence::PresenceState; -use time; use config::Config; use db::DB; @@ -14,7 +13,7 @@ use middleware::{AccessTokenAuth, JsonRequest, MiddlewareChain, UserIdParam}; use modifier::SerializableResponse; use models::room_membership::RoomMembership; use models::presence_list::PresenceList; -use models::presence_status::PresenceStatus; +use models::presence_status::{PresenceStatus, get_now}; use models::user::User; /// The PUT `/presence/:user_id/status` endpoint. @@ -78,7 +77,7 @@ struct GetPresenceStatusResponse { /// 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, + last_active_ago: i64, /// This user's presence. One of: ["online", "offline", "unavailable"] presence: PresenceState, } @@ -94,7 +93,7 @@ impl Handler for GetPresenceStatus { let connection = DB::from_request(request)?; if user.id != user_id { - let rooms = RoomMembership::find_common_joined_rooms( + let rooms = RoomMembership::find_common_rooms( &connection, &user.id, &user_id, @@ -102,7 +101,7 @@ impl Handler for GetPresenceStatus { )?; if rooms.is_empty() { Err(ApiError::unauthorized( - format!("You are not authorized to get the presence status for th given user_id: {}.", user_id) + format!("You are not authorized to get the presence status for the given user_id: {}.", user_id) ))?; } } @@ -117,14 +116,13 @@ impl Handler for GetPresenceStatus { let presence_state: PresenceState = status.presence.parse() .expect("Database insert should ensure a PresenceState"); - let now = time::get_time(); - let last_update = time::Timespec::new(status.updated_at.0, 0); - let last_active_ago: time::Duration = last_update - now; + let now = get_now(); + let last_active_ago = now - status.updated_at.0; let response = GetPresenceStatusResponse { status_msg: status.status_msg, currently_active: PresenceState::Online == presence_state, - last_active_ago: last_active_ago.num_milliseconds() as u64, + last_active_ago: last_active_ago, presence: presence_state, }; @@ -202,9 +200,13 @@ impl Handler for GetPresenceList { #[cfg(test)] mod tests { - use test::Test; + use std::thread; + use std::time::Duration; + use iron::status::Status; + use test::Test; + #[test] fn basic_presence_status() { let test = Test::new(); @@ -435,4 +437,48 @@ mod tests { let array = response.json().as_array().unwrap(); assert_eq!(array.len(), 0); } + + #[test] + fn last_active_ago() { + let test = Test::new(); + let alice = test.create_user(); + let bob = test.create_user(); + let carl = test.create_user(); + + let room_options = format!(r#"{{"invite": ["{}", "{}"]}}"#, bob.id, carl.id); + let room_id = test.create_room_with_params(&alice.token, &room_options); + + assert_eq!(test.join_room(&bob.token, &room_id).status, Status::Ok); + assert_eq!(test.join_room(&carl.token, &room_id).status, Status::Ok); + + test.update_presence(&alice.token, &alice.id, r#"{"presence":"online"}"#); + thread::sleep(Duration::from_secs(2)); + + test.update_presence(&bob.token, &bob.id, r#"{"presence":"online"}"#); + thread::sleep(Duration::from_secs(2)); + + let alice_presence_path = format!( + "/_matrix/client/r0/presence/{}/status?access_token={}", + alice.id, + carl.token + ); + + let bob_presence_path = format!( + "/_matrix/client/r0/presence/{}/status?access_token={}", + bob.id, + carl.token + ); + + let bob_response = test.get(&bob_presence_path); + assert_eq!(bob_response.status, Status::Ok); + let last_active_ago = bob_response.json().find("last_active_ago").unwrap().as_u64().unwrap(); + assert!(last_active_ago > 2_000); + assert!(last_active_ago < 2_500); + + let alice_response = test.get(&alice_presence_path); + assert_eq!(alice_response.status, Status::Ok); + let last_active_ago = alice_response.json().find("last_active_ago").unwrap().as_u64().unwrap(); + assert!(last_active_ago > 4_000); + assert!(last_active_ago < 4_500); + } } diff --git a/src/api/r0/profile.rs b/src/api/r0/profile.rs index 7fda3f0d..6125a3c0 100644 --- a/src/api/r0/profile.rs +++ b/src/api/r0/profile.rs @@ -239,8 +239,6 @@ mod tests { use test::Test; use iron::status::Status; use query::SyncOptions; - use std::time::Duration; - use std::thread; #[test] fn get_displayname_non_existent_user() { @@ -510,9 +508,6 @@ mod tests { timeout: 0 }; - // The precision is in seconds. - thread::sleep(Duration::from_secs(2)); - let response = test.sync(&carl.token, options); let array = response .json() @@ -560,9 +555,6 @@ mod tests { timeout: 0 }; - // The precision is in seconds. - thread::sleep(Duration::from_secs(2)); - let response = test.sync(&carl.token, options); let array = response .json() @@ -596,9 +588,6 @@ mod tests { timeout: 0 }; - // The precision is in seconds. - thread::sleep(Duration::from_secs(2)); - let response = test.sync(&carl.token, options); let array = response .json() diff --git a/src/main.rs b/src/main.rs index 51462f4f..c2e45277 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,7 +29,6 @@ extern crate serde; #[macro_use] extern crate serde_derive; extern crate serde_json; extern crate serde_yaml; -extern crate time; extern crate toml; extern crate unicase; extern crate url; diff --git a/src/models/presence_list.rs b/src/models/presence_list.rs index e85501f0..1125493a 100644 --- a/src/models/presence_list.rs +++ b/src/models/presence_list.rs @@ -17,10 +17,9 @@ use diesel::pg::PgConnection; use ruma_events::EventType; use ruma_events::presence::{PresenceEvent, PresenceEventContent, PresenceState}; use ruma_identifiers::UserId; -use time; use error::ApiError; -use models::presence_status::PresenceStatus; +use models::presence_status::{PresenceStatus, get_now}; use models::profile::Profile; use models::room_membership::RoomMembership; use models::user::User; @@ -88,7 +87,7 @@ impl PresenceList { let mut invites: Vec = Vec::new(); for ref observed_user in invite.clone() { if observed_user != user_id { - let rooms = RoomMembership::get_common_rooms( + let rooms = RoomMembership::filter_rooms_by_state( connection, &room_ids, observed_user, @@ -136,7 +135,7 @@ impl PresenceList { pub fn find_events_by_uid( connection: &PgConnection, user_id: &UserId, - since: Option + since: Option ) -> Result<(i64, Vec), ApiError> { let mut max_ordering = -1; @@ -149,11 +148,11 @@ impl PresenceList { let mut events = Vec::new(); for status in users_status { - let last_update = time::Timespec::new(status.updated_at.0, 0); - max_ordering = cmp::max(last_update.sec, max_ordering); + let last_update = status.updated_at.0; + max_ordering = cmp::max(last_update, max_ordering); let presence_state: PresenceState = status.presence.parse().unwrap(); - let last_active_ago: time::Duration = last_update - time::get_time(); + let last_active_ago = get_now() - last_update; let profile: Option<&Profile> = profiles.iter() .filter(|profile| profile.id == status.user_id) @@ -172,7 +171,7 @@ impl PresenceList { avatar_url: avatar_url, currently_active: PresenceState::Online == presence_state, displayname: displayname, - last_active_ago: Some(last_active_ago.num_milliseconds() as u64), + last_active_ago: Some(last_active_ago as u64), presence: presence_state, user_id: status.user_id, }, diff --git a/src/models/presence_status.rs b/src/models/presence_status.rs index 6020fd42..831ff72c 100644 --- a/src/models/presence_status.rs +++ b/src/models/presence_status.rs @@ -1,5 +1,6 @@ //! Storage and querying of presence status. +use chrono::{Duration, NaiveDateTime, NaiveDate, UTC}; use diesel::{ insert, Connection, @@ -16,7 +17,6 @@ use diesel::pg::data_types::PgTimestamp; use diesel::result::Error as DieselError; use ruma_events::presence::PresenceState; use ruma_identifiers::{UserId, EventId}; -use time; use error::ApiError; use schema::presence_status; @@ -32,7 +32,9 @@ pub struct NewPresenceStatus { /// The current presence state. pub presence: String, /// A possible status message from the user. - pub status_msg: Option + pub status_msg: Option, + /// Timestamp of the last update. + pub updated_at: PgTimestamp, } /// A Matrix presence status. @@ -52,6 +54,19 @@ pub struct PresenceStatus { pub updated_at: PgTimestamp, } +/// Return current time in milliseconds +pub fn get_now() -> i64 { + let now = UTC::now().naive_utc(); + get_milliseconds(now) +} + +/// Return `time` in milliseconds with a same epoch as PostgreSQL. +pub fn get_milliseconds(time: NaiveDateTime) -> i64 { + let epoch = NaiveDate::from_ymd(2000, 1, 1).and_hms(0, 0, 0); + let duration: Duration = time - epoch; + duration.num_milliseconds() +} + impl PresenceStatus { /// Update or insert a presence status entry. pub fn upsert( @@ -91,9 +106,7 @@ impl PresenceStatus { self.presence = presence; self.status_msg = status_msg; self.event_id = event_id.clone(); - - // Use seconds instead of microseconds (default for PgTimestamp) - self.updated_at = PgTimestamp(time::get_time().sec); + self.updated_at = PgTimestamp(get_now()); match self.save_changes::(connection) { Ok(_) => Ok(()), @@ -114,6 +127,7 @@ impl PresenceStatus { event_id: event_id.clone(), presence: presence, status_msg: status_msg, + updated_at: PgTimestamp(get_now()), }; insert(&new_status) .into(presence_status::table) @@ -141,11 +155,11 @@ impl PresenceStatus { pub fn get_users( connection: &PgConnection, users: &Vec, - since: Option, + since: Option, ) -> Result, ApiError> { match since { Some(since) => { - let time = PgTimestamp(since.sec); + let time = PgTimestamp(since); presence_status::table .filter(presence_status::user_id.eq(any(users))) diff --git a/src/models/room_membership.rs b/src/models/room_membership.rs index a37f49dc..8e4fbe0d 100644 --- a/src/models/room_membership.rs +++ b/src/models/room_membership.rs @@ -405,7 +405,7 @@ impl RoomMembership { } /// Return `RoomId`'s for given `UserId`'s. - pub fn find_common_joined_rooms( + pub fn find_common_rooms( connection: &PgConnection, user_id: &UserId, observed_user_id: &UserId, @@ -425,8 +425,8 @@ impl RoomMembership { .map_err(ApiError::from) } - /// Return `RoomId`'s for given `RoomId`'s and `UserId`. - pub fn get_common_rooms( + /// Filter `RoomId`'s for `UserId` and membership state. + pub fn filter_rooms_by_state( connection: &PgConnection, room_ids: &Vec, user_id: &UserId, diff --git a/src/query.rs b/src/query.rs index b6a5e59f..f5d734e0 100644 --- a/src/query.rs +++ b/src/query.rs @@ -17,7 +17,6 @@ use ruma_events::presence::PresenceState; use ruma_events::collections::all::{RoomEvent, StateEvent}; use ruma_identifiers::RoomId; use serde_json::Value; -use time; use error::ApiError; use models::event::Event; @@ -208,7 +207,7 @@ impl Sync { let since = match *context { Context::Incremental(batch) | Context::FullState(batch) => { - Some(time::Timespec::new(batch.presence_key, 0)) + Some(batch.presence_key) } Context::Initial => None, };