From cda31fb7553ba3d880de09a464ae3b62ea6632fc Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 3 Sep 2014 15:37:10 +0100 Subject: [PATCH] Kill the state ... key from all the Presence messages --- synapse/handlers/presence.py | 31 +++--------------- synapse/rest/presence.py | 6 +--- tests/handlers/test_presence.py | 50 ++++++++++------------------- tests/handlers/test_presencelike.py | 47 ++++++++++++++------------- tests/rest/test_presence.py | 20 +++++++----- 5 files changed, 60 insertions(+), 94 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index a88ce61f7..e911e1851 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -180,7 +180,7 @@ class PresenceHandler(BaseHandler): state = yield self.store.get_presence_state(target_user.localpart) if "mtime" in state: del state["mtime"] - state["presence"] = state["state"] + state["presence"] = state.pop("state") if target_user in self._user_cachemap: state["last_active"] = ( @@ -213,15 +213,11 @@ class PresenceHandler(BaseHandler): state["status_msg"] = None for k in state.keys(): - if k not in ("presence", "state", "status_msg"): + if k not in ("presence", "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") - if state["presence"] not in self.STATE_LEVELS: raise SynapseError(400, "'%s' is not a valid presence state" % state["presence"] @@ -600,7 +596,7 @@ class PresenceHandler(BaseHandler): if state is None: state = yield self.store.get_presence_state(user.localpart) del state["mtime"] - state["presence"] = state["state"] + state["presence"] = state.pop("state") if user in self._user_cachemap: state["last_active"] = ( @@ -621,8 +617,6 @@ 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, @@ -654,21 +648,12 @@ class PresenceHandler(BaseHandler): state = dict(push) del state["user_id"] - if "presence" in state: - # all is OK - pass - elif "state" in state: - # Legacy handling - state["presence"] = state["state"] - else: + if "presence" not in state: logger.warning("Received a presence 'push' EDU from %s without" - + " either a 'presence' or 'state' key", origin + + " a 'presence' key", origin ) continue - if "state" in state: - del state["state"] - if "last_active_ago" in state: state["last_active"] = int( self.clock.time_msec() - state.pop("last_active_ago") @@ -900,7 +885,6 @@ 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' @@ -918,11 +902,6 @@ class UserPresenceCache(object): def get_state(self): # clone it so caller can't break our cache state = dict(self.state) - - # Legacy handling - if "presence" in state: - state["state"] = state["presence"] - return state def make_event(self, user, clock): diff --git a/synapse/rest/presence.py b/synapse/rest/presence.py index 5c5adb423..9410fcae5 100644 --- a/synapse/rest/presence.py +++ b/synapse/rest/presence.py @@ -51,11 +51,7 @@ class PresenceStatusRestServlet(RestServlet): try: content = json.loads(request.content.read()) - # Legacy handling - if "state" in content: - state["presence"] = content.pop("state") - else: - state["presence"] = content.pop("presence") + 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 8506961d2..74ac3ea8f 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -142,7 +142,7 @@ class PresenceStateTestCase(unittest.TestCase): ) self.assertEquals( - {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, + {"presence": ONLINE, "status_msg": "Online"}, state ) mocked_get.assert_called_with("apple") @@ -159,7 +159,7 @@ class PresenceStateTestCase(unittest.TestCase): ) self.assertEquals( - {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, + {"presence": ONLINE, "status_msg": "Online"}, state ) mocked_get.assert_called_with("apple") @@ -178,7 +178,7 @@ class PresenceStateTestCase(unittest.TestCase): ) self.assertEquals( - {"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, + {"presence": ONLINE, "status_msg": "Online"}, state ) @@ -208,7 +208,8 @@ class PresenceStateTestCase(unittest.TestCase): state={"presence": UNAVAILABLE, "status_msg": "Away"}) mocked_set.assert_called_with("apple", - {"state": UNAVAILABLE, "status_msg": "Away"}) + {"state": UNAVAILABLE, "status_msg": "Away"} + ) self.mock_start.assert_called_with(self.u_apple, state={ "presence": UNAVAILABLE, @@ -460,8 +461,7 @@ class PresenceInvitesTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, - "presence": OFFLINE, - "state": OFFLINE}, + "presence": OFFLINE}, ], presence) self.datastore.get_presence_list.assert_called_with("apple", @@ -478,8 +478,7 @@ class PresenceInvitesTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, - "presence": OFFLINE, - "state": OFFLINE}, + "presence": OFFLINE}, ], presence) self.datastore.get_presence_list.assert_called_with("apple", @@ -625,7 +624,8 @@ class PresencePushTestCase(unittest.TestCase): self.room_members = [self.u_apple, self.u_elderberry] self.datastore.set_presence_state.return_value = defer.succeed( - {"state": ONLINE}) + {"state": ONLINE} + ) # TODO(paul): Gut-wrenching self.handler._user_cachemap[self.u_apple] = UserPresenceCache() @@ -654,7 +654,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@apple:test", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 0, }}, ], @@ -673,7 +672,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@apple:test", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 0, }}, ], @@ -692,7 +690,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@apple:test", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 0, }}, ], @@ -715,11 +712,9 @@ class PresencePushTestCase(unittest.TestCase): self.assertEquals( [ {"observed_user": self.u_banana, - "presence": OFFLINE, - "state": OFFLINE}, + "presence": OFFLINE}, {"observed_user": self.u_clementine, - "presence": OFFLINE, - "state": OFFLINE}, + "presence": OFFLINE}, ], presence ) @@ -740,11 +735,9 @@ class PresencePushTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, "presence": ONLINE, - "state": ONLINE, "last_active_ago": 2000}, {"observed_user": self.u_clementine, - "presence": OFFLINE, - "state": OFFLINE}, + "presence": OFFLINE}, ], presence) (events, _) = yield self.event_source.get_new_events_for_user( @@ -758,7 +751,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@banana:test", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 2000 }}, ] @@ -775,7 +767,6 @@ class PresencePushTestCase(unittest.TestCase): "push": [ {"user_id": "@apple:test", "presence": u"online", - "state": u"online", "last_active_ago": 0}, ], } @@ -791,7 +782,6 @@ class PresencePushTestCase(unittest.TestCase): "push": [ {"user_id": "@apple:test", "presence": u"online", - "state": u"online", "last_active_ago": 0}, ], } @@ -837,7 +827,7 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@potato:remote", - "state": "online", + "presence": "online", "last_active_ago": 1000}, ], } @@ -855,7 +845,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@potato:remote", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 1000, }} ] @@ -866,7 +855,7 @@ class PresencePushTestCase(unittest.TestCase): state = yield self.handler.get_state(self.u_potato, self.u_apple) self.assertEquals( - {"state": ONLINE, "presence": ONLINE, "last_active_ago": 3000}, + {"presence": ONLINE, "last_active_ago": 3000}, state ) @@ -902,7 +891,6 @@ class PresencePushTestCase(unittest.TestCase): "content": { "user_id": "@clementine:test", "presence": ONLINE, - "state": ONLINE, "last_active_ago": 0, }} ] @@ -919,8 +907,7 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@apple:test", - "presence": "online", - "state": "online"}, + "presence": "online"}, ], } ), @@ -934,8 +921,7 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@banana:test", - "presence": "offline", - "state": "offline"}, + "presence": "offline"}, ], } ), @@ -964,8 +950,7 @@ class PresencePushTestCase(unittest.TestCase): content={ "push": [ {"user_id": "@clementine:test", - "presence": "online", - "state": "online"}, + "presence": "online"}, ], } ), @@ -1264,7 +1249,6 @@ class PresencePollingTestCase(unittest.TestCase): "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 638e506d3..fc0ef8c70 100644 --- a/tests/handlers/test_presencelike.py +++ b/tests/handlers/test_presencelike.py @@ -148,10 +148,11 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): yield self.handlers.presence_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"}) + {"state": UNAVAILABLE, "status_msg": "Away"} + ) @defer.inlineCallbacks def test_push_local(self): @@ -161,7 +162,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): ] self.datastore.set_presence_state.return_value = defer.succeed( - {"state": ONLINE}) + {"state": ONLINE} + ) # TODO(paul): Gut-wrenching from synapse.handlers.presence import UserPresenceCache @@ -177,9 +179,11 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): apple_set.add(self.u_clementine) yield self.handlers.presence_handler.set_state(self.u_apple, - self.u_apple, {"state": ONLINE}) + self.u_apple, {"presence": ONLINE} + ) yield self.handlers.presence_handler.set_state(self.u_banana, - self.u_banana, {"state": ONLINE}) + self.u_banana, {"presence": ONLINE} + ) presence = yield self.handlers.presence_handler.get_presence_list( observer_user=self.u_apple, accepted=True) @@ -187,14 +191,12 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, "presence": ONLINE, - "state": ONLINE, "last_active_ago": 0, "displayname": "Frank", "avatar_url": "http://foo"}, {"observed_user": self.u_clementine, - "presence": OFFLINE, - "state": OFFLINE}], - presence) + "presence": OFFLINE} + ], presence) self.mock_update_client.assert_has_calls([ call(users_to_push=set([self.u_apple, self.u_banana, self.u_clementine]), @@ -242,7 +244,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): ] self.datastore.set_presence_state.return_value = defer.succeed( - {"state": ONLINE}) + {"state": ONLINE} + ) # TODO(paul): Gut-wrenching from synapse.handlers.presence import UserPresenceCache @@ -257,7 +260,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): apple_set.add(self.u_potato.domain) yield self.handlers.presence_handler.set_state(self.u_apple, - self.u_apple, {"state": ONLINE}) + self.u_apple, {"presence": ONLINE} + ) self.replication.send_edu.assert_called_with( destination="remote", @@ -266,7 +270,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): "push": [ {"user_id": "@apple:test", "presence": "online", - "state": "online", "last_active_ago": 0, "displayname": "Frank", "avatar_url": "http://foo"}, @@ -283,18 +286,19 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): # TODO(paul): Gut-wrenching potato_set = self.handlers.presence_handler._remote_recvmap.setdefault( - self.u_potato, set()) + self.u_potato, set() + ) potato_set.add(self.u_apple) yield self.replication.received_edu( - "remote", "m.presence", { - "push": [ - {"user_id": "@potato:remote", - "state": "online", - "displayname": "Frank", - "avatar_url": "http://foo"}, - ], - } + "remote", "m.presence", { + "push": [ + {"user_id": "@potato:remote", + "presence": "online", + "displayname": "Frank", + "avatar_url": "http://foo"}, + ], + } ) self.mock_update_client.assert_called_with( @@ -313,7 +317,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase): 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 df8dd7415..f2751f56d 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -99,7 +99,7 @@ class PresenceStateTestCase(unittest.TestCase): self.assertEquals(200, code) self.assertEquals( - {"presence": ONLINE, "state": ONLINE, "status_msg": "Available"}, + {"presence": ONLINE, "status_msg": "Available"}, response ) mocked_get.assert_called_with("apple") @@ -115,7 +115,8 @@ class PresenceStateTestCase(unittest.TestCase): self.assertEquals(200, code) mocked_set.assert_called_with("apple", - {"state": UNAVAILABLE, "status_msg": "Away"}) + {"state": UNAVAILABLE, "status_msg": "Away"} + ) class PresenceListTestCase(unittest.TestCase): @@ -176,7 +177,7 @@ class PresenceListTestCase(unittest.TestCase): self.assertEquals(200, code) self.assertEquals([ - {"user_id": "@banana:test", "presence": OFFLINE, "state": OFFLINE}, + {"user_id": "@banana:test", "presence": OFFLINE}, ], response) self.datastore.get_presence_list.assert_called_with( @@ -311,9 +312,11 @@ class PresenceEventStreamTestCase(unittest.TestCase): self.room_members = [self.u_apple, self.u_banana] self.mock_datastore.set_presence_state.return_value = defer.succeed( - {"state": ONLINE}) + {"state": ONLINE} + ) self.mock_datastore.get_presence_list.return_value = defer.succeed( - []) + [] + ) (code, response) = yield self.mock_resource.trigger("GET", "/events?timeout=0", None) @@ -329,9 +332,11 @@ class PresenceEventStreamTestCase(unittest.TestCase): ) self.mock_datastore.set_presence_state.return_value = defer.succeed( - {"state": ONLINE}) + {"state": ONLINE} + ) self.mock_datastore.get_presence_list.return_value = defer.succeed( - []) + [] + ) yield self.presence.set_state(self.u_banana, self.u_banana, state={"presence": ONLINE} @@ -346,7 +351,6 @@ class PresenceEventStreamTestCase(unittest.TestCase): "content": { "user_id": "@banana:test", "presence": ONLINE, - "state": ONLINE, "displayname": "Frank", "last_active_ago": 0, }},