Kill the state

... key from all the Presence messages

5 files changed, 60 insertions, 94 deletions

diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py
index a88ce61f79..e911e18511 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 5c5adb4236..9410fcae56 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 8506961d2d..74ac3ea8f5 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 638e506d34..fc0ef8c705 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 df8dd74151..f2751f56d3 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,
             }},