From 801a551da14385d814c048565a5f05c220888404 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 3 Sep 2014 17:04:58 +0100 Subject: [PATCH] Rename 'mtime' DB field to 'last_active', adjusted semantics --- synapse/handlers/presence.py | 19 ++++++++----------- synapse/storage/presence.py | 6 ++---- synapse/storage/schema/delta/v3.sql | 7 ++++--- synapse/storage/schema/presence.sql | 2 +- tests/handlers/test_presence.py | 10 +++++++--- tests/handlers/test_presencelike.py | 4 +++- tests/rest/test_presence.py | 7 +++++-- 7 files changed, 30 insertions(+), 25 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 7a8ebf7954..5b4a107c63 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -178,8 +178,6 @@ class PresenceHandler(BaseHandler): if not visible: raise SynapseError(404, "Presence information not visible") state = yield self.store.get_presence_state(target_user.localpart) - if "mtime" in state: - del state["mtime"] if target_user in self._user_cachemap: state["last_active"] = ( @@ -225,12 +223,17 @@ class PresenceHandler(BaseHandler): logger.debug("Updating presence state of %s to %s", target_user.localpart, state["presence"]) - state_to_store = dict(state) - statuscache=self._get_or_offline_usercache(target_user) - was_level = self.STATE_LEVELS[statuscache.get_state()["presence"]] + oldstate = statuscache.get_state() + + was_level = self.STATE_LEVELS[oldstate["presence"]] now_level = self.STATE_LEVELS[state["presence"]] + if now_level > was_level: + state["last_active"] = self.clock.time_msec() + + state_to_store = dict(state) + yield defer.DeferredList([ self.store.set_presence_state( target_user.localpart, state_to_store @@ -240,9 +243,6 @@ class PresenceHandler(BaseHandler): ), ]) - if now_level > was_level: - state["last_active"] = self.clock.time_msec() - now_online = state["presence"] != PresenceState.OFFLINE was_polling = target_user in self._user_cachemap @@ -593,7 +593,6 @@ class PresenceHandler(BaseHandler): def _push_presence_remote(self, user, destination, state=None): if state is None: state = yield self.store.get_presence_state(user.localpart) - del state["mtime"] if user in self._user_cachemap: state["last_active"] = ( @@ -881,8 +880,6 @@ class UserPresenceCache(object): self.serial = None def update(self, state, serial): - assert("mtime_age" not in state) - self.state.update(state) # Delete keys that are now 'None' for k in self.state.keys(): diff --git a/synapse/storage/presence.py b/synapse/storage/presence.py index da40c0454c..0d628a3c8d 100644 --- a/synapse/storage/presence.py +++ b/synapse/storage/presence.py @@ -35,16 +35,14 @@ class PresenceStore(SQLBaseStore): return self._simple_select_one( table="presence", keyvalues={"user_id": user_localpart}, - retcols=["presence", "status_msg", "mtime"], + retcols=["presence", "status_msg", "last_active"], ) def set_presence_state(self, user_localpart, new_state): return self._simple_update_one( table="presence", keyvalues={"user_id": user_localpart}, - updatevalues={"presence": new_state["presence"], - "status_msg": new_state["status_msg"], - "mtime": self._clock.time_msec()}, + updatevalues=new_state, retcols=["presence"], ) diff --git a/synapse/storage/schema/delta/v3.sql b/synapse/storage/schema/delta/v3.sql index 589de8e14e..42b0ae64e8 100644 --- a/synapse/storage/schema/delta/v3.sql +++ b/synapse/storage/schema/delta/v3.sql @@ -20,12 +20,13 @@ CREATE TABLE NEW_presence( user_id INTEGER NOT NULL, presence INTEGER, status_msg TEXT, - mtime INTEGER, + last_active INTEGER, FOREIGN KEY(user_id) REFERENCES users(id) ); --- rename the 'state' field to 'presence' -INSERT INTO NEW_presence (user_id, presence, status_msg, mtime) +-- rename the 'state' field to 'presence'; migrate the old 'mtime' field into +-- the new 'last_active' field +INSERT INTO NEW_presence (user_id, presence, status_msg, last_active) SELECT user_id, state, status_msg, mtime FROM presence; DROP TABLE presence; diff --git a/synapse/storage/schema/presence.sql b/synapse/storage/schema/presence.sql index e8736ee87b..c45ec774cd 100644 --- a/synapse/storage/schema/presence.sql +++ b/synapse/storage/schema/presence.sql @@ -16,7 +16,7 @@ CREATE TABLE IF NOT EXISTS presence( user_id INTEGER NOT NULL, presence INTEGER, status_msg TEXT, - mtime INTEGER, -- miliseconds since last state change + last_active INTEGER, FOREIGN KEY(user_id) REFERENCES users(id) ); diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 9efc35376a..5ae7739c2d 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -208,7 +208,10 @@ class PresenceStateTestCase(unittest.TestCase): state={"presence": UNAVAILABLE, "status_msg": "Away"}) mocked_set.assert_called_with("apple", - {"presence": UNAVAILABLE, "status_msg": "Away"} + {"presence": UNAVAILABLE, + "status_msg": "Away", + "last_active": 1000000, # MockClock + } ) self.mock_start.assert_called_with(self.u_apple, state={ @@ -1045,7 +1048,7 @@ class PresencePollingTestCase(unittest.TestCase): return defer.succeed( {"presence": self.current_user_state[user_localpart], "status_msg": None, - "mtime": 123456000} + "last_active": 500000} ) self.datastore.get_presence_state = get_presence_state @@ -1249,7 +1252,8 @@ class PresencePollingTestCase(unittest.TestCase): "push": [ {"user_id": "@banana:test", "presence": "offline", - "status_msg": None}, + "status_msg": None, + "last_active_ago": 500000}, ], }, ), diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index 2f551f1b6b..7abf0fd5f8 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -151,7 +151,9 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): state={"presence": UNAVAILABLE, "status_msg": "Away"}) mocked_set.assert_called_with("apple", - {"presence": UNAVAILABLE, "status_msg": "Away"} + {"presence": UNAVAILABLE, + "status_msg": "Away", + "last_active": 1000000} ) @defer.inlineCallbacks diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index f355bcf712..9b0f44e637 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -21,7 +21,7 @@ from twisted.internet import defer from mock import Mock import logging -from ..utils import MockHttpResource +from ..utils import MockHttpResource, MockClock from synapse.api.constants import PresenceState from synapse.handlers.presence import PresenceHandler @@ -51,6 +51,7 @@ class PresenceStateTestCase(unittest.TestCase): self.mock_resource = MockHttpResource(prefix=PATH_PREFIX) hs = HomeServer("test", + clock=MockClock(), db_pool=None, datastore=Mock(spec=[ "get_presence_state", @@ -115,7 +116,9 @@ class PresenceStateTestCase(unittest.TestCase): self.assertEquals(200, code) mocked_set.assert_called_with("apple", - {"presence": UNAVAILABLE, "status_msg": "Away"} + {"presence": UNAVAILABLE, + "status_msg": "Away", + "last_active": 1000000} )