Add basic implementation for the presence endpoints. #137

farodin91 · 2016-12-28T12:02:27Z

State:

Add Endpoints
Fix migrations of table filters
Add only existing users (presence_list)
Support presence in sync with since
Support sync set_presence
Change some sync response types to event_type collections
Add config for custom presence timeout
Clean up RoomMembership by moving a user existence test to User
Update Status.md
Update ruma-events to 0.3.0
Add check before getting status endpoint. (Alice and Bob must be in a same Room.)
Add check before updating list endpoint. (Alice and Bob must be in a same Room.)
Sending a m.presence event again after changing avatar_url or displayname

ToDo

Update last_active_ago and currently_active to work as the spec says.
Create a server behavior to update presence state if they are too old.
Support limit in sync presence

Fixes

farodin91 · 2016-12-28T22:49:12Z

@mujx I get an error 'UserIdParam should ensure a UserId' during the test and i have no idea why this happen. Any idea?

farodin91 · 2016-12-29T12:09:51Z

I forgot ::chain() in server.rs

mujx · 2017-01-05T11:30:30Z

I don't think this is ready for review because there are some test failures. The sync error is not related to the deadlocks.

farodin91 · 2017-01-05T11:59:10Z

@mujx Fixed

mujx

Looks good 👍

On a side note I don't think it's necessary to have three files for the presence endpoints.

mujx · 2017-01-05T12:27:18Z

migrations/001_prerelease/up.sql

+    presence TEXT NOT NULL,
+    status_msg TEXT,
+    updated_at TIMESTAMP NOT NULL DEFAULT now(),
+    UNIQUE (id)


You already have id as the primary key.

mujx · 2017-01-05T12:27:36Z

migrations/001_prerelease/up.sql

@@ -93,3 +93,27 @@ CREATE TABLE filters (
    content TEXT NOT NULL,
    UNIQUE (id, user_id)
 );
+
+CREATE TABLE presence_status (
+    id TEXT NOT NULL PRIMARY KEY,


NOT NULL is implied. Also rename it to user_id since it's not obvious unless you read the code.

If i rename this to use_id, i can't use Identifable.

You can use #[primary_key(user_id)]. Check out the changelog in diesel.

mujx · 2017-01-05T12:28:25Z

migrations/001_prerelease/up.sql

+CREATE TABLE  presence_list (
+    user_id TEXT NOT NULL,
+    observed_user_id TEXT NOT NULL,
+    UNIQUE (user_id, observed_user_id)


You can use them as composite primary key with PRIMARY KEY(user_id, observed_user_id)

mujx · 2017-01-05T12:29:24Z

migrations/001_prerelease/up.sql

+    UNIQUE (user_id, observed_user_id)
+);
+
+CREATE TABLE presence_stream (


Same thing with presence_info.

nit: Would it make more sense to rename it to presence_events because it's the same thing with the events table.

mujx · 2017-01-05T13:02:40Z

src/models/presence_status.rs

+    pub updated_at: SystemTime,
+}
+
+fn to_string(state: PresenceState) -> String {


I have opened a PR on ruma-events for this.

https://github.com/ruma/ruma-events/releases/tag/0.3.0

mujx · 2017-01-05T15:29:45Z

src/models/presence_list.rs

+                .map_err(ApiError::from)?
+        };
+
+        let profiles: Vec<Profile> = profiles::table


You can move this into profiles too.

I think of adding profiles to the presence_stream table reduce requests and clean up the code.

Can you post here the schema you are proposing?

CREATE TABLE presence_events ( ordering BIGSERIAL NOT NULL, event_id TEXT PRIMARY KEY, user_id TEXT NOT NULL, presence TEXT NOT NULL, avatar_url TEXT, displayname TEXT, created_at TIMESTAMP NOT NULL DEFAULT now(), UNIQUE (ordering) );

mujx · 2017-01-05T15:58:29Z

src/api/r0/sync.rs


        let url = request.url.clone().into_generic_url();
        let query_pairs = url.query_pairs().into_owned();

        let mut filter = None;
        let mut since = None;
        let mut full_state = false;
-        let mut set_presence = PresenceState::Offline;
+        let mut set_presence = None;


The default seems to be offline

Spec is inconsequence.

No problem. You can keep it offline though to reduce the diff because it's not really necessary.

Agreed here, let's default to offline rather than making it an Option.

I looked deeper into synapse and they use Online as default.

They updated it in the head version.

Controls whether the client is automatically marked as online by polling this API. If this parameter is omitted then the client is automatically marked as online when it uses this API. Otherwise if the parameter is set to "offline" then the client is not marked as being online when it uses this API. One of: ["offline"]

mujx · 2017-01-05T16:00:43Z

src/api/r0/sync.rs

@@ -107,7 +113,7 @@ mod tests {
            filter: None,
            since: None,
            full_state: false,
-            set_presence: PresenceState::Offline,
+            set_presence: None,


Not sure why you changed those.

Same here, let's switch back to offline.

mujx · 2017-01-05T16:03:40Z

src/api/r0/sync.rs

+            .unwrap()
+            .as_array()
+            .unwrap();
+        assert_eq!(array.len(), 2);


Maybe show here the contents of the events.

mujx · 2017-01-05T16:17:10Z

src/models/presence_list.rs

+    }
+
+    /// Return `PresenceEvent`'s for given `UserId`.
+    pub fn find_events(connection: &PgConnection, user_id: &UserId, since: Option<i64>)


Maybe find_events_per_user? Because find_events seems too generic.

farodin91 · 2017-01-06T17:59:11Z

Adding server behavior of presence in different PR request.
Updating presence state after sometime to unavailable.

farodin91 · 2017-01-06T19:31:37Z

@mujx I'm done with refactoring the requests. I'm open an issue due to the not resolved request.

mujx · 2017-01-06T19:35:48Z

src/test.rs

@@ -96,6 +96,7 @@ impl Test {
            domain: "ruma.test".to_string(),
            macaroon_secret_key: "YymznQHmKdN9B4f7iBalJB1tWEDy9LdaFSQJEtB3R5w=".into(),
            postgres_url: DATABASE_URL.to_string(),
+            update_interval_presence: 3000000,


I'm not really convinced that it's necessary to put this as a config variable. It seems like something that should be the same among the homeservers. @jimmycuadra thoughts?

mujx · 2017-01-06T20:00:23Z

src/models/user.rs

@@ -117,6 +123,46 @@ impl User {
            Err(error) => Err(ApiError::from(error)),
        }
    }
+
+    pub fn find_missing_user_and_check_existence(


Missing docs (Run make ci locally to avoid these errors).
nit: You can put all the arguments in the same line.

I don't think the naming is correct here. You provide a list of users and you want to know which are valid but you also do the error handling. Also you have hardcoded the error message and it would be the wrong one for the dropped users. You will need a test for that.

You can rename the function to find_missing_users which returns a vector with the missing ones (if any). Then you check if the returned vector is not empty and you throw the appropriate error. Use !missing_users.is_empty() like clippy suggests.

mujx · 2017-01-06T20:19:42Z

src/models/profile.rs

@@ -126,4 +130,17 @@ impl Profile {
            Err(err) => Err(ApiError::from(err)),
        }
    }
+
+    pub fn find_for_presence_list_by_uid(


Missing docs. Also the name is a little confusing.

You can split this into a function PresenceList::get_observed_user_list(UserId) or get_observed_users for the first query and another function Profile::get_profiles(Vec<UserId>) to retrieve the profiles.

The problem is Diesel limited. You can not do something like this Diesel, without sending multiple request to database. I don't like my solution, but it is better than sending to request.
Performance!!!

mujx · 2017-01-06T20:25:01Z

src/models/presence_status.rs

+        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 {


Same lines with presence_status.

I removed this part incorrect behavior.

mujx · 2017-01-06T20:56:05Z

src/models/presence_event.rs

+            .map_err(ApiError::from)
+    }
+
+    pub fn find_for_presence_list_by_uid(


Missing docs

mujx · 2017-01-06T21:01:33Z

src/models/presence_event.rs

+            .filter(presence_list::user_id.eq(user_id))
+            .select(presence_list::observed_user_id);
+
+        if let Some(since) = since {


Here you have essentially the same code in the if else blocks. You can refactor this into a function and pass since as an Option.

The problem is Diesel limited. You can not do something like this Diesel, without sending multiple request to database. I don't like my solution, but it is better than sending to request.

mujx · 2017-01-08T17:50:17Z

migrations/001_prerelease/up.sql

+    event_id TEXT PRIMARY KEY,
+    user_id TEXT NOT NULL,
+    presence TEXT NOT NULL,
+    avatar_url TEXT,


That seems unnecessary. We already have tables for profiles.

Done because the code is much cleaner. I don't have to build a complex code to combine these data. I could revert this easy.

The problem with this is synchronization. You already have an avatar and display name when you save the presence event. Then you update your profile info but the presence event that you need to return has the old info. Updating the events with the new profile info seems awkward.

There are other events so that the client can learn about profile changes. So unless I'm missing something I don't see the point of sending/save them. Also synapse doesn't seem to include them.

Also I'm not so sure about the presence_events table. Having a full history of these events doesn't have a use case (synapse deletes old entries) because we only need the latest ones. I was thinking about moving that info into presence_list and then only updating those entries.

I like to split this into multiple request. We have to add queue to clean up and update events. I try to move it into presence_status, but i make it complicated to work with ordering. Saving the profile in presence_list. Help to speed up the critical endpoint sync, see synapse. Normally user doesn't change their profile often.

http://matrix.org/speculator/spec/HEAD/client_server/unstable.html#events-on-change-of-profile-information

👍 for the link, that cleared things a bit. My main objection is about having the displayname and avatar_url into two separate tables. If you want these fields you just use the profiles table. You already have the user_id so it would be fast enough.

I will return to this ruma/ruma@fcb617a#diff-bb665a0481f7bd2c8bedef7b2dd9f702L117. If you are okay with it.

mujx · 2017-01-08T17:55:03Z

src/api/r0/presence.rs

+        let connection = DB::from_request(request)?;
+
+        let status = PresenceStatus::find_by_uid(&connection, &user_id)?;
+        let status: PresenceStatus = match status {


Just do match find_by_uid here.

mujx · 2017-01-08T17:59:18Z

src/api/r0/presence.rs

+        };
+
+        let presence_state: PresenceState = status.presence.parse()
+            .expect("Something wrong with the database!");


Not really a database problem. Probably error while parsing presence state.

This shouldn't happened anytime.

mujx · 2017-01-08T18:11:23Z

src/models/presence_status.rs

+    }
+
+    /// Calculate the difference between two SystemTimes in milliseconds.
+    pub fn calculate_last_active_ago(since: SystemTime, now: SystemTime) -> Result<u64, ApiError> {


Should probably be renamed to calculate_time_difference.

jimmycuadra · 2017-01-14T01:11:09Z

What's the status here? Are the items on the todo list in the PR description still to be done as part of this PR, or are those going to be in another PR?

farodin91 · 2017-01-14T14:58:11Z

@mujx Can you look into two latest commits?
@jimmycuadra I could implement limit. And we create issues for all others. Some needs a bit discussions.

I will update tests to the new schema, before i squash or resolve issues.

mujx · 2017-01-14T16:53:54Z

Rebase on the current master. Also please try not to include any more changes in this PR. Generally prefer smaller consice changes that are easy to review and merge (for example the sync stuff could have been updated in a separate PR).

farodin91 · 2017-01-14T17:34:35Z

My next PR will be smaller.

mujx

In models/precence_events.rs we always use the latest events (max
ordering etc). This is natural because those events give us the most recent
info about the user's precence status which is what we want. Accessing older
entries for a user wouldn't make any sense because they contain outdated
information.

Given that these precence_events don't have any historical
value we can remove the table and just keep the latest info inside the
presence_status entries (which essentialy is the same table). I have made some changes (as a proof of concept) on @farodin91 's branch to show my point. Should we do something like that or keep the current logic?

mujx · 2017-01-14T18:21:50Z

migrations/001_prerelease/down.sql

@@ -8,3 +8,7 @@ DROP TABLE room_memberships;
 DROP TABLE rooms;
 DROP TABLE users;
 DROP TABLE room_tags;
+DROP TABLE filters;


Could this be related to #140?

I don't think so.

mujx · 2017-01-14T18:22:58Z

src/api/r0/presence.rs

+use bodyparser;
+use iron::status::Status;
+use iron::{Chain, Handler, IronResult, IronError, Plugin, Request, Response};
+use ruma_identifiers::{UserId};


use ruma_identifiers::UserId;

mujx · 2017-01-14T18:33:03Z

src/api/r0/presence.rs

+
+        let put_presence_status_request = match request.get::<bodyparser::Struct<PutPresenceStatusRequest>>() {
+            Ok(Some(request)) => request,
+            Ok(None) | Err(_) => {


Ok(None) | Err(_) => Err(ApiError::bad_json(None))?,

mujx · 2017-01-14T18:43:44Z

src/models/presence_status.rs

@@ -0,0 +1,172 @@
+//! Storage and querying of presence status.
+
+#[cfg(test)]


You don't need this line.

I need Duration only for the test.

Oh right didn't see that.

mujx · 2017-01-14T18:52:07Z

src/api/r0/presence.rs

+        if user.id != user_id {
+            let rooms = RoomMembership::find_shared_rooms_by_uid(&connection, &user.id, &user_id)?;
+            if rooms.is_empty() {
+                return Err(IronError::from(


Err(ApiError::unauthorized( format!("...", user_id) ))?;

to make the line a little shorter.

mujx · 2017-01-15T17:33:01Z

src/models/profile.rs

@@ -126,4 +148,18 @@ impl Profile {
            Err(err) => Err(ApiError::from(err)),
        }
    }
+
+    /// Return `Profile`s for given `UserId` and his `PresenceList` entries.
+    pub fn find_profiles_by_presence_list(


This could be a generic get_profiles(connection: &PgConnection, users: Vec<UserId>) so it's not specific to the presence list.

mujx · 2017-01-15T22:00:14Z

src/models/presence_status.rs

+        let mut presence_state = PresenceState::Unavailable;
+        let mut status_msg = None;
+
+        match PresenceStatus::find_by_uid(connection, user_id)? {


You don't need this. You already call this inside upsert.

mujx · 2017-01-15T22:01:15Z

src/models/profile.rs

-                Profile::create(connection, &new_profile)
-            }
-        }
+            PresenceStatus::update_by_uid_and_status(connection, homeserver_domain, &user_id)?;


Call PresenceStatus::upsert directly here.

mujx · 2017-01-15T22:01:33Z

src/models/profile.rs

-                Profile::create(connection, &new_profile)
-            }
-        }
+            PresenceStatus::update_by_uid_and_status(connection, homeserver_domain, &user_id)?;


Call PresenceStatus::upsert directly here.

mujx · 2017-01-15T22:16:24Z

src/api/r0/profile.rs

+            .as_array()
+            .unwrap();
+        let mut events = array.into_iter();
+        assert_eq!(events.len(), 1);


If I understand correctly this one entry is his own presence, because this user doesn't observe anyone else. Is this the correct behavior?

farodin91 · 2017-01-15T23:09:52Z

@mujx That's look good. Could you push it in my branch? I'll give you access.

farodin91 · 2017-01-15T23:16:08Z

Or open an PR i'll merge it.

mujx · 2017-01-15T23:20:59Z

No need for write access. You can just add .patch to the url and git apply in your branch.

# State: - Add Endpoints - Fix migrations of table filters - Add only existing users (presence_list) - Support presence in sync with `since` - Support sync `set_presence` - Change some sync response types to event_type collections - Add config for custom presence timeout - Clean up `RoomMembership` by moving a user existence test to `User` - Update Status.md - Update `ruma-events` to 0.3.0 - Add check before getting `status` endpoint. (Alice and Bob must be in a same Room.) - Add check before updating `list` endpoint. (Alice and Bob must be in a same Room.) - Sending a `m.presence` event again after changing `avatar_url` or `displayname` # Fixes - ruma#39 - ruma#40 - ruma#41 - ruma#42

mujx

I found a bug on my previous patch and some other minor things.

edit: TIL the diesel timestamp is actually microseconds since January 1st 2000 not UNIX time .

mujx · 2017-01-16T23:00:06Z

src/api/r0/presence.rs

+            )?;
+            if rooms.is_empty() {
+                Err(ApiError::unauthorized(
+                    format!("You are not authorized to get the presence status for th given user_id: {}.", user_id)


th given => the given

mujx · 2017-01-16T23:00:48Z

src/api/r0/presence.rs

+        let connection = DB::from_request(request)?;
+
+        if user.id != user_id {
+            let rooms = RoomMembership::find_common_joined_rooms(


Rename to find_common_rooms because you already pass "join" as a parameter. Or remove the membership parameter and keep the name.

mujx · 2017-01-16T23:09:26Z

src/models/profile.rs

-                Profile::create(connection, &new_profile)
-            }
-        }
+            PresenceStatus::upsert(connection, homeserver_domain, &user_id, None, None)?;


Shouldn't this be set to online by default?

No.

a m.presence presence status update is sent, again containing the new values of the displayname and avatar_url keys, in addition to the required presence key containing the current presence state of the user.

mujx · 2017-01-16T23:13:32Z

src/models/room_membership.rs

+    }
+
+    /// Return `RoomId`'s for given `UserId`'s.
+    pub fn find_common_joined_rooms(


This should be renamed to find_common_rooms.

mujx · 2017-01-17T12:43:09Z

src/api/r0/presence.rs

+
+        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;


This should be the other way around. i.e now - last_update. Actually there is a bug here. When a new presence status entry is created in the database the default PgTimestamp format is used which is in microseconds and only when we update it we start using seconds. I would suggest to keep the default microseconds format for PgTimestamp and just convert to secs etc.

We also need a test to verify that the last_active_ago parameter in the response is correct.

I would use every one time. Server or Database time.

mujx · 2017-01-17T13:22:32Z

src/api/r0/profile.rs

+        };
+
+        // The precision is in seconds.
+        thread::sleep(Duration::from_secs(2));


We probably don't need those if we keep the microseconds timestamp.

mujx · 2017-01-17T13:24:52Z

src/models/presence_status.rs

+        self.event_id = event_id.clone();
+
+        // Use seconds instead of microseconds (default for PgTimestamp)
+        self.updated_at = PgTimestamp(time::get_time().sec);


This will be added automatically.

I'm only using datetimes generated by the server.

mujx · 2017-01-17T13:25:28Z

src/models/presence_status.rs

+    ) -> Result<Vec<PresenceStatus>, ApiError> {
+        match since {
+            Some(since) => {
+                let time = PgTimestamp(since.sec);


We need microseconds here

mujx · 2017-01-17T13:28:19Z

src/models/room_membership.rs

+    }
+
+    /// Return `RoomId`'s for given `RoomId`'s and `UserId`.
+    pub fn get_common_rooms(


Maybe rename it to filter_rooms_by_state?. You only have one user so the naming is a little off.

mujx

Great job 👍

mujx · 2017-01-17T21:35:21Z

src/models/presence_status.rs

@@ -52,6 +54,20 @@ pub struct PresenceStatus {
    pub updated_at: PgTimestamp,
 }

+/// Return now


Maybe Return current time in milliseconds?

mujx · 2017-01-17T21:35:35Z

src/models/presence_status.rs

@@ -52,6 +54,20 @@ pub struct PresenceStatus {
    pub updated_at: PgTimestamp,
 }

+/// Return now
+pub fn get_now() -> i64 {
+    // Use seconds instead of microseconds (default for PgTimestamp)


You can remove this comment.

farodin91 · 2017-02-03T19:01:04Z

@jimmycuadra Any progress?

jimmycuadra · 2017-02-04T01:06:07Z

Sorry I've left this sitting! I'll review it within a couple days.

farodin91 added component/api spec/presence status/work in progress type/feature labels Dec 28, 2016

farodin91 force-pushed the presence branch 5 times, most recently from 5900a73 to eb5c515 Compare January 4, 2017 19:47

farodin91 changed the title ~~WIP: Add basic implementation for the presence endpoints.~~ Add basic implementation for the presence endpoints. Jan 4, 2017

farodin91 requested review from jimmycuadra and mujx January 4, 2017 19:49

farodin91 added status/needs review and removed status/work in progress labels Jan 4, 2017

mujx suggested changes Jan 5, 2017

View reviewed changes

farodin91 force-pushed the presence branch from 9a166d6 to b8be1b8 Compare January 6, 2017 18:01

mujx suggested changes Jan 6, 2017

View reviewed changes

farodin91 force-pushed the presence branch 2 times, most recently from b26f225 to 11fec19 Compare January 8, 2017 15:33

mujx suggested changes Jan 8, 2017

View reviewed changes

farodin91 force-pushed the presence branch 2 times, most recently from 57125a4 to eb6e591 Compare January 10, 2017 22:02

farodin91 force-pushed the presence branch from eb6e591 to 55b56a0 Compare January 14, 2017 15:53

mujx suggested changes Jan 15, 2017

View reviewed changes

farodin91 force-pushed the presence branch from 0657a5d to b7c200f Compare January 16, 2017 22:37

mujx suggested changes Jan 17, 2017

View reviewed changes

mujx mentioned this pull request Jan 17, 2017

Switch from PgTimestamp to SystemTime. #152

Open

mujx approved these changes Jan 17, 2017

View reviewed changes

farodin91 force-pushed the presence branch from ae282b1 to 010c5b4 Compare January 17, 2017 21:41

Fixing some timing issues and resolve some typos

010c5b4

jimmycuadra approved these changes Feb 6, 2017

View reviewed changes

jimmycuadra merged commit b4709d6 into ruma:master Feb 6, 2017

farodin91 deleted the presence branch June 22, 2017 20:33

		@@ -0,0 +1,172 @@
		//! Storage and querying of presence status.

		#[cfg(test)]

Add basic implementation for the presence endpoints. #137

Add basic implementation for the presence endpoints. #137

Conversation

farodin91 commented Dec 28, 2016 • edited Loading

State:

ToDo

Fixes

farodin91 commented Dec 28, 2016

farodin91 commented Dec 29, 2016

mujx commented Jan 5, 2017

farodin91 commented Jan 5, 2017

mujx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farodin91 commented Jan 6, 2017

farodin91 commented Jan 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mujx Jan 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mujx Jan 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mujx Jan 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmycuadra commented Jan 14, 2017

farodin91 commented Jan 14, 2017

mujx commented Jan 14, 2017

farodin91 commented Jan 14, 2017

mujx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farodin91 commented Dec 28, 2016 •

edited

Loading

mujx Jan 8, 2017 •

edited

Loading

mujx Jan 10, 2017 •

edited

Loading

mujx Jan 8, 2017 •

edited

Loading

mujx left a comment •

edited

Loading

mujx Jan 15, 2017 •

edited

Loading

mujx left a comment •

edited

Loading