From 02f4e3b3ff613a6e9024c0fef416be0bf92bf48f Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 1 Sep 2014 14:45:35 +0100 Subject: [PATCH] Rename 'state' presence key to the much more obvious 'presence'; maintain a legacy 'state' copy for now --- docs/client-server/specification.rst | 4 +- docs/specification.rst | 14 +-- synapse/handlers/presence.py | 42 ++++++--- synapse/rest/presence.py | 6 +- tests/handlers/test_presence.py | 122 +++++++++++++++++---------- tests/handlers/test_presencelike.py | 19 +++-- tests/rest/test_presence.py | 18 ++-- 7 files changed, 147 insertions(+), 78 deletions(-) diff --git a/docs/client-server/specification.rst b/docs/client-server/specification.rst index ee8bb5c42..4c9e313a6 100644 --- a/docs/client-server/specification.rst +++ b/docs/client-server/specification.rst @@ -1026,7 +1026,7 @@ Getting/Setting your own presence state REST Path: /presence/$user_id/status Valid methods: GET/PUT Required keys: - state : [0|1|2|3] - The user's new presence state + presence : [0|1|2|3] - The user's new presence state Optional keys: status_msg : text string provided by the user to explain their status @@ -1039,7 +1039,7 @@ Fetching your presence list following keys: { "user_id" : string giving the observed user's ID - "state" : int giving their status + "presence" : int giving their status "status_msg" : optional text string "displayname" : optional text string from the user's profile "avatar_url" : optional text string from the user's profile diff --git a/docs/specification.rst b/docs/specification.rst index b61994ff3..d47705ca0 100644 --- a/docs/specification.rst +++ b/docs/specification.rst @@ -542,7 +542,7 @@ Each user has the concept of presence information. This encodes the "availability" of that user, suitable for display on other user's clients. This is transmitted as an ``m.presence`` event and is one of the few events which are sent *outside the context of a room*. The basic piece of presence information -is represented by the ``state`` key, which is an enum of one of the following: +is represented by the ``presence`` key, which is an enum of one of the following: - ``online`` : The default state when the user is connected to an event stream. - ``unavailable`` : The user is not reachable at this time. @@ -552,18 +552,18 @@ is represented by the ``state`` key, which is an enum of one of the following: - ``hidden`` : TODO. Behaves as offline, but allows the user to see the client state anyway and generally interact with client features. -This basic ``state`` field applies to the user as a whole, regardless of how many +This basic ``presence`` field applies to the user as a whole, regardless of how many client devices they have connected. The home server should synchronise this status choice among multiple devices to ensure the user gets a consistent experience. Idle Time --------- -As well as the basic ``state`` field, the presence information can also show a sense -of an "idle timer". This should be maintained individually by the user's -clients, and the home server can take the highest reported time as that to -report. When a user is offline, the home server can still report when the user was last -seen online. +As well as the basic ``presence`` field, the presence information can also show +a sense of an "idle timer". This should be maintained individually by the +user's clients, and the home server can take the highest reported time as that +to report. When a user is offline, the home server can still report when the +user was last seen online. Transmission ------------ diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 93bd07b19..fa5942ba0 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -135,7 +135,7 @@ class PresenceHandler(BaseHandler): return self._user_cachemap[user] else: statuscache = UserPresenceCache() - statuscache.update({"state": PresenceState.OFFLINE}, user) + statuscache.update({"presence": PresenceState.OFFLINE}, user) return statuscache def registered_user(self, user): @@ -177,6 +177,7 @@ class PresenceHandler(BaseHandler): state = yield self.store.get_presence_state( target_user.localpart ) + state["presence"] = state["state"] else: raise SynapseError(404, "Presence information not visible") else: @@ -207,15 +208,20 @@ class PresenceHandler(BaseHandler): state["status_msg"] = None for k in state.keys(): - if k not in ("state", "status_msg"): + if k not in ("presence", "state", "status_msg"): raise SynapseError( 400, "Unexpected presence state key '%s'" % (k,) ) + # Handle legacy "state" key for now + if "state" in state: + state["presence"] = state.pop("state") + logger.debug("Updating presence state of %s to %s", - target_user.localpart, state["state"]) + target_user.localpart, state["presence"]) state_to_store = dict(state) + state_to_store["state"] = state_to_store.pop("presence") yield defer.DeferredList([ self.store.set_presence_state( @@ -228,7 +234,7 @@ class PresenceHandler(BaseHandler): state["mtime"] = self.clock.time_msec() - now_online = state["state"] != PresenceState.OFFLINE + now_online = state["presence"] != PresenceState.OFFLINE was_polling = target_user in self._user_cachemap if now_online and not was_polling: @@ -251,12 +257,12 @@ class PresenceHandler(BaseHandler): @log_function def started_user_eventstream(self, user): # TODO(paul): Use "last online" state - self.set_state(user, user, {"state": PresenceState.ONLINE}) + self.set_state(user, user, {"presence": PresenceState.ONLINE}) @log_function def stopped_user_eventstream(self, user): # TODO(paul): Save current state as "last online" state - self.set_state(user, user, {"state": PresenceState.OFFLINE}) + self.set_state(user, user, {"presence": PresenceState.OFFLINE}) @defer.inlineCallbacks def user_joined_room(self, user, room_id): @@ -576,6 +582,7 @@ 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) + state["presence"] = state["state"] yield self.distributor.fire( "collect_presencelike_data", user, state @@ -591,6 +598,8 @@ class PresenceHandler(BaseHandler): "user_id": user.to_string(), } user_state.update(**state) + if "state" in user_state and "presence" not in user_state: + user_state["presence"] = user_state["state"] yield self.federation.send_edu( destination=destination, @@ -622,6 +631,11 @@ class PresenceHandler(BaseHandler): state = dict(push) del state["user_id"] + # Legacy handling + if "presence" not in state: + state["presence"] = state["state"] + del state["state"] + if "mtime_age" in state: state["mtime"] = int( self.clock.time_msec() - state.pop("mtime_age") @@ -639,7 +653,7 @@ class PresenceHandler(BaseHandler): statuscache=statuscache, ) - if state["state"] == PresenceState.OFFLINE: + if state["presence"] == PresenceState.OFFLINE: del self._user_cachemap[user] for poll in content.get("poll", []): @@ -672,10 +686,9 @@ class PresenceHandler(BaseHandler): yield defer.DeferredList(deferreds) @defer.inlineCallbacks - def push_update_to_local_and_remote(self, observed_user, + def push_update_to_local_and_remote(self, observed_user, statuscache, users_to_push=[], room_ids=[], - remote_domains=[], - statuscache=None): + remote_domains=[]): localusers, remoteusers = partitionbool( users_to_push, @@ -804,6 +817,7 @@ class UserPresenceCache(object): def update(self, state, serial): assert("mtime_age" not in state) + assert("state" not in state) self.state.update(state) # Delete keys that are now 'None' @@ -820,7 +834,13 @@ class UserPresenceCache(object): def get_state(self): # clone it so caller can't break our cache - return dict(self.state) + state = dict(self.state) + + # Legacy handling + if "presence" in state: + state["state"] = state["presence"] + + return state def make_event(self, user, clock): content = self.get_state() diff --git a/synapse/rest/presence.py b/synapse/rest/presence.py index e013b2085..bce394354 100644 --- a/synapse/rest/presence.py +++ b/synapse/rest/presence.py @@ -48,7 +48,11 @@ class PresenceStatusRestServlet(RestServlet): try: content = json.loads(request.content.read()) - state["state"] = content.pop("state") + # Legacy handling + if "state" in content: + state["presence"] = content.pop("state") + else: + state["presence"] = content.pop("presence") if "status_msg" in content: state["status_msg"] = content.pop("status_msg") diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 451e1eaa2..f37d9917f 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -35,8 +35,6 @@ ONLINE = PresenceState.ONLINE logging.getLogger().addHandler(logging.NullHandler()) -#logging.getLogger().addHandler(logging.StreamHandler()) -#logging.getLogger().setLevel(logging.DEBUG) def _expect_edu(destination, edu_type, content, origin="test"): @@ -141,7 +139,8 @@ class PresenceStateTestCase(unittest.TestCase): target_user=self.u_apple, auth_user=self.u_apple ) - self.assertEquals({"state": ONLINE, "status_msg": "Online"}, + self.assertEquals( + {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, state ) mocked_get.assert_called_with("apple") @@ -157,7 +156,8 @@ class PresenceStateTestCase(unittest.TestCase): target_user=self.u_apple, auth_user=self.u_banana ) - self.assertEquals({"state": ONLINE, "status_msg": "Online"}, + self.assertEquals( + {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, state ) mocked_get.assert_called_with("apple") @@ -175,7 +175,10 @@ class PresenceStateTestCase(unittest.TestCase): target_user=self.u_apple, auth_user=self.u_clementine ) - self.assertEquals({"state": ONLINE, "status_msg": "Online"}, state) + self.assertEquals( + {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, + state + ) @defer.inlineCallbacks def test_get_disallowed_state(self): @@ -202,20 +205,20 @@ class PresenceStateTestCase(unittest.TestCase): yield self.handler.set_state( target_user=self.u_apple, auth_user=self.u_apple, - state={"state": UNAVAILABLE, "status_msg": "Away"}) + state={"presence": UNAVAILABLE, "status_msg": "Away"}) mocked_set.assert_called_with("apple", {"state": UNAVAILABLE, "status_msg": "Away"}) self.mock_start.assert_called_with(self.u_apple, state={ - "state": UNAVAILABLE, + "presence": UNAVAILABLE, "status_msg": "Away", "mtime": 1000000, # MockClock }) yield self.handler.set_state( target_user=self.u_apple, auth_user=self.u_apple, - state={"state": OFFLINE}) + state={"presence": OFFLINE}) self.mock_stop.assert_called_with(self.u_apple) @@ -449,28 +452,35 @@ class PresenceInvitesTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_presence_list(self): self.datastore.get_presence_list.return_value = defer.succeed( - [{"observed_user_id": "@banana:test"}] + [{"observed_user_id": "@banana:test"}] ) presence = yield self.handler.get_presence_list( observer_user=self.u_apple) - self.assertEquals([{"observed_user": self.u_banana, - "state": OFFLINE}], presence) + self.assertEquals([ + {"observed_user": self.u_banana, + "presence": OFFLINE, + "state": OFFLINE}, + ], presence) self.datastore.get_presence_list.assert_called_with("apple", - accepted=None) - + accepted=None + ) self.datastore.get_presence_list.return_value = defer.succeed( - [{"observed_user_id": "@banana:test"}] + [{"observed_user_id": "@banana:test"}] ) presence = yield self.handler.get_presence_list( - observer_user=self.u_apple, accepted=True) + observer_user=self.u_apple, accepted=True + ) - self.assertEquals([{"observed_user": self.u_banana, - "state": OFFLINE}], presence) + self.assertEquals([ + {"observed_user": self.u_banana, + "presence": OFFLINE, + "state": OFFLINE}, + ], presence) self.datastore.get_presence_list.assert_called_with("apple", accepted=True) @@ -618,7 +628,8 @@ class PresencePushTestCase(unittest.TestCase): self.assertEquals(self.event_source.get_current_key(), 0) yield self.handler.set_state(self.u_apple, self.u_apple, - {"state": ONLINE}) + {"presence": ONLINE} + ) self.assertEquals(self.event_source.get_current_key(), 1) self.assertEquals( @@ -627,6 +638,7 @@ class PresencePushTestCase(unittest.TestCase): {"type": "m.presence", "content": { "user_id": "@apple:test", + "presence": ONLINE, "state": ONLINE, "mtime_age": 0, }}, @@ -636,13 +648,21 @@ class PresencePushTestCase(unittest.TestCase): presence = yield self.handler.get_presence_list( observer_user=self.u_apple, accepted=True) - self.assertEquals([ - {"observed_user": self.u_banana, "state": OFFLINE}, - {"observed_user": self.u_clementine, "state": OFFLINE}], - presence) + self.assertEquals( + [ + {"observed_user": self.u_banana, + "presence": OFFLINE, + "state": OFFLINE}, + {"observed_user": self.u_clementine, + "presence": OFFLINE, + "state": OFFLINE}, + ], + presence + ) yield self.handler.set_state(self.u_banana, self.u_banana, - {"state": ONLINE}) + {"presence": ONLINE} + ) self.clock.advance_time(2) @@ -651,9 +671,11 @@ class PresencePushTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, + "presence": ONLINE, "state": ONLINE, "mtime_age": 2000}, {"observed_user": self.u_clementine, + "presence": OFFLINE, "state": OFFLINE}, ], presence) @@ -666,6 +688,7 @@ class PresencePushTestCase(unittest.TestCase): {"type": "m.presence", "content": { "user_id": "@banana:test", + "presence": ONLINE, "state": ONLINE, "mtime_age": 2000 }}, @@ -682,6 +705,7 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@apple:test", + "presence": u"online", "state": u"online", "mtime_age": 0}, ], @@ -703,7 +727,7 @@ class PresencePushTestCase(unittest.TestCase): apple_set.add(self.u_potato.domain) yield self.handler.set_state(self.u_apple, self.u_apple, - {"state": ONLINE} + {"presence": ONLINE} ) yield put_json.await_calls() @@ -741,6 +765,7 @@ class PresencePushTestCase(unittest.TestCase): {"type": "m.presence", "content": { "user_id": "@potato:remote", + "presence": ONLINE, "state": ONLINE, "mtime_age": 1000, }} @@ -751,7 +776,10 @@ class PresencePushTestCase(unittest.TestCase): state = yield self.handler.get_state(self.u_potato, self.u_apple) - self.assertEquals({"state": ONLINE, "mtime_age": 3000}, state) + self.assertEquals( + {"state": ONLINE, "presence": ONLINE, "mtime_age": 3000}, + state + ) @defer.inlineCallbacks def test_join_room_local(self): @@ -763,7 +791,7 @@ class PresencePushTestCase(unittest.TestCase): self.handler._user_cachemap[self.u_clementine] = UserPresenceCache() self.handler._user_cachemap[self.u_clementine].update( { - "state": PresenceState.ONLINE, + "presence": PresenceState.ONLINE, "mtime": self.clock.time_msec(), }, self.u_clementine ) @@ -781,6 +809,7 @@ class PresencePushTestCase(unittest.TestCase): {"type": "m.presence", "content": { "user_id": "@clementine:test", + "presence": ONLINE, "state": ONLINE, "mtime_age": 0, }} @@ -798,7 +827,8 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@apple:test", - "state": "online"}, + "presence": "online", + "state": "online"}, ], } ), @@ -812,7 +842,8 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@banana:test", - "state": "offline"}, + "presence": "offline", + "state": "offline"}, ], } ), @@ -823,7 +854,7 @@ class PresencePushTestCase(unittest.TestCase): # TODO(paul): Gut-wrenching self.handler._user_cachemap[self.u_apple] = UserPresenceCache() self.handler._user_cachemap[self.u_apple].update( - {"state": PresenceState.ONLINE}, self.u_apple) + {"presence": PresenceState.ONLINE}, self.u_apple) self.room_members = [self.u_apple, self.u_banana] yield self.distributor.fire("user_joined_room", self.u_potato, @@ -841,7 +872,8 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@clementine:test", - "state": "online"}, + "presence": "online", + "state": "online"}, ], } ), @@ -851,7 +883,7 @@ class PresencePushTestCase(unittest.TestCase): self.handler._user_cachemap[self.u_clementine] = UserPresenceCache() self.handler._user_cachemap[self.u_clementine].update( - {"state": ONLINE}, self.u_clementine) + {"presence": ONLINE}, self.u_clementine) self.room_members.append(self.u_potato) yield self.distributor.fire("user_joined_room", self.u_clementine, @@ -969,7 +1001,7 @@ class PresencePollingTestCase(unittest.TestCase): # apple goes online yield self.handler.set_state( target_user=self.u_apple, auth_user=self.u_apple, - state={"state": ONLINE} + state={"presence": ONLINE} ) # apple should see both banana and clementine currently offline @@ -992,8 +1024,9 @@ class PresencePollingTestCase(unittest.TestCase): # banana goes online yield self.handler.set_state( - target_user=self.u_banana, auth_user=self.u_banana, - state={"state": ONLINE}) + target_user=self.u_banana, auth_user=self.u_banana, + state={"presence": ONLINE} + ) # apple and banana should now both see each other online self.mock_update_client.assert_has_calls([ @@ -1013,8 +1046,9 @@ class PresencePollingTestCase(unittest.TestCase): # apple goes offline yield self.handler.set_state( - target_user=self.u_apple, auth_user=self.u_apple, - state={"state": OFFLINE}) + target_user=self.u_apple, auth_user=self.u_apple, + state={"presence": OFFLINE} + ) # banana should now be told apple is offline self.mock_update_client.assert_has_calls([ @@ -1027,7 +1061,6 @@ class PresencePollingTestCase(unittest.TestCase): self.assertFalse("banana" in self.handler._local_pushmap) self.assertFalse("clementine" in self.handler._local_pushmap) - @defer.inlineCallbacks def test_remote_poll_send(self): put_json = self.mock_http_client.put_json @@ -1057,8 +1090,9 @@ class PresencePollingTestCase(unittest.TestCase): # clementine goes online yield self.handler.set_state( - target_user=self.u_clementine, auth_user=self.u_clementine, - state={"state": ONLINE}) + target_user=self.u_clementine, auth_user=self.u_clementine, + state={"presence": ONLINE} + ) yield put_json.await_calls() @@ -1085,7 +1119,7 @@ class PresencePollingTestCase(unittest.TestCase): # fig goes online; shouldn't send a second poll yield self.handler.set_state( target_user=self.u_fig, auth_user=self.u_fig, - state={"state": ONLINE} + state={"presence": ONLINE} ) # reactor.iterate(delay=0) @@ -1095,7 +1129,7 @@ class PresencePollingTestCase(unittest.TestCase): # fig goes offline yield self.handler.set_state( target_user=self.u_fig, auth_user=self.u_fig, - state={"state": OFFLINE} + state={"presence": OFFLINE} ) reactor.iterate(delay=0) @@ -1116,8 +1150,9 @@ class PresencePollingTestCase(unittest.TestCase): # clementine goes offline yield self.handler.set_state( - target_user=self.u_clementine, auth_user=self.u_clementine, - state={"state": OFFLINE}) + target_user=self.u_clementine, auth_user=self.u_clementine, + state={"presence": OFFLINE} + ) yield put_json.await_calls() @@ -1135,6 +1170,7 @@ class PresencePollingTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@banana:test", + "presence": "offline", "state": "offline", "status_msg": None}, ], diff --git a/tests/handlers/test_presencelike.py b/tests/handlers/test_presencelike.py index e81d7ce10..2402bed8d 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -182,11 +182,13 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, + "presence": ONLINE, "state": ONLINE, "mtime_age": 0, "displayname": "Frank", "avatar_url": "http://foo"}, {"observed_user": self.u_clementine, + "presence": OFFLINE, "state": OFFLINE}], presence) @@ -199,7 +201,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): statuscache = self.mock_update_client.call_args[1]["statuscache"] self.assertEquals({ - "state": ONLINE, + "presence": ONLINE, "mtime": 1000000, # MockClock "displayname": "Frank", "avatar_url": "http://foo", @@ -222,7 +224,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): statuscache = self.mock_update_client.call_args[1]["statuscache"] self.assertEquals({ - "state": ONLINE, + "presence": ONLINE, "mtime": 1000000, # MockClock "displayname": "I am an Apple", "avatar_url": "http://foo", @@ -255,6 +257,7 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@apple:test", + "presence": "online", "state": "online", "mtime_age": 0, "displayname": "Frank", @@ -293,14 +296,16 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): statuscache=ANY) statuscache = self.mock_update_client.call_args[1]["statuscache"] - self.assertEquals({"state": ONLINE, + self.assertEquals({"presence": ONLINE, "displayname": "Frank", "avatar_url": "http://foo"}, statuscache.state) state = yield self.handlers.presence_handler.get_state(self.u_potato, self.u_apple) - self.assertEquals({"state": ONLINE, - "displayname": "Frank", - "avatar_url": "http://foo"}, - state) + self.assertEquals( + {"presence": ONLINE, + "state": ONLINE, + "displayname": "Frank", + "avatar_url": "http://foo"}, + state) diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index 61692f02c..f5afa4a25 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -98,8 +98,10 @@ class PresenceStateTestCase(unittest.TestCase): "/presence/%s/status" % (myid), None) self.assertEquals(200, code) - self.assertEquals({"state": ONLINE, "status_msg": "Available"}, - response) + self.assertEquals( + {"presence": ONLINE, "state": ONLINE, "status_msg": "Available"}, + response + ) mocked_get.assert_called_with("apple") @defer.inlineCallbacks @@ -109,7 +111,7 @@ class PresenceStateTestCase(unittest.TestCase): (code, response) = yield self.mock_resource.trigger("PUT", "/presence/%s/status" % (myid), - '{"state": "unavailable", "status_msg": "Away"}') + '{"presence": "unavailable", "status_msg": "Away"}') self.assertEquals(200, code) mocked_set.assert_called_with("apple", @@ -173,9 +175,9 @@ class PresenceListTestCase(unittest.TestCase): "/presence/list/%s" % (myid), None) self.assertEquals(200, code) - self.assertEquals( - [{"user_id": "@banana:test", "state": OFFLINE}], response - ) + self.assertEquals([ + {"user_id": "@banana:test", "presence": OFFLINE, "state": OFFLINE}, + ], response) self.datastore.get_presence_list.assert_called_with( "apple", accepted=True @@ -314,7 +316,8 @@ class PresenceEventStreamTestCase(unittest.TestCase): []) yield self.presence.set_state(self.u_banana, self.u_banana, - state={"state": ONLINE}) + state={"presence": ONLINE} + ) (code, response) = yield self.mock_resource.trigger("GET", "/events?from=0_1_0&timeout=0", None) @@ -324,6 +327,7 @@ class PresenceEventStreamTestCase(unittest.TestCase): {"type": "m.presence", "content": { "user_id": "@banana:test", + "presence": ONLINE, "state": ONLINE, "displayname": "Frank", "mtime_age": 0,