Rename 'mtime' DB field to 'last_active', adjusted semantics

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}
         )