Kill the state

... key from all the Presence messages
This commit is contained in:
Paul "LeoNerd" Evans 2014-09-03 15:37:10 +01:00
parent dada11dc5f
commit cda31fb755
5 changed files with 60 additions and 94 deletions

View file

@ -180,7 +180,7 @@ class PresenceHandler(BaseHandler):
state = yield self.store.get_presence_state(target_user.localpart) state = yield self.store.get_presence_state(target_user.localpart)
if "mtime" in state: if "mtime" in state:
del state["mtime"] del state["mtime"]
state["presence"] = state["state"] state["presence"] = state.pop("state")
if target_user in self._user_cachemap: if target_user in self._user_cachemap:
state["last_active"] = ( state["last_active"] = (
@ -213,15 +213,11 @@ class PresenceHandler(BaseHandler):
state["status_msg"] = None state["status_msg"] = None
for k in state.keys(): for k in state.keys():
if k not in ("presence", "state", "status_msg"): if k not in ("presence", "status_msg"):
raise SynapseError( raise SynapseError(
400, "Unexpected presence state key '%s'" % (k,) 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: if state["presence"] not in self.STATE_LEVELS:
raise SynapseError(400, "'%s' is not a valid presence state" % raise SynapseError(400, "'%s' is not a valid presence state" %
state["presence"] state["presence"]
@ -600,7 +596,7 @@ class PresenceHandler(BaseHandler):
if state is None: if state is None:
state = yield self.store.get_presence_state(user.localpart) state = yield self.store.get_presence_state(user.localpart)
del state["mtime"] del state["mtime"]
state["presence"] = state["state"] state["presence"] = state.pop("state")
if user in self._user_cachemap: if user in self._user_cachemap:
state["last_active"] = ( state["last_active"] = (
@ -621,8 +617,6 @@ class PresenceHandler(BaseHandler):
"user_id": user.to_string(), "user_id": user.to_string(),
} }
user_state.update(**state) 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( yield self.federation.send_edu(
destination=destination, destination=destination,
@ -654,21 +648,12 @@ class PresenceHandler(BaseHandler):
state = dict(push) state = dict(push)
del state["user_id"] del state["user_id"]
if "presence" in state: if "presence" not in state:
# all is OK
pass
elif "state" in state:
# Legacy handling
state["presence"] = state["state"]
else:
logger.warning("Received a presence 'push' EDU from %s without" logger.warning("Received a presence 'push' EDU from %s without"
+ " either a 'presence' or 'state' key", origin + " a 'presence' key", origin
) )
continue continue
if "state" in state:
del state["state"]
if "last_active_ago" in state: if "last_active_ago" in state:
state["last_active"] = int( state["last_active"] = int(
self.clock.time_msec() - state.pop("last_active_ago") self.clock.time_msec() - state.pop("last_active_ago")
@ -900,7 +885,6 @@ class UserPresenceCache(object):
def update(self, state, serial): def update(self, state, serial):
assert("mtime_age" not in state) assert("mtime_age" not in state)
assert("state" not in state)
self.state.update(state) self.state.update(state)
# Delete keys that are now 'None' # Delete keys that are now 'None'
@ -918,11 +902,6 @@ class UserPresenceCache(object):
def get_state(self): def get_state(self):
# clone it so caller can't break our cache # clone it so caller can't break our cache
state = dict(self.state) state = dict(self.state)
# Legacy handling
if "presence" in state:
state["state"] = state["presence"]
return state return state
def make_event(self, user, clock): def make_event(self, user, clock):

View file

@ -51,11 +51,7 @@ class PresenceStatusRestServlet(RestServlet):
try: try:
content = json.loads(request.content.read()) content = json.loads(request.content.read())
# Legacy handling state["presence"] = content.pop("presence")
if "state" in content:
state["presence"] = content.pop("state")
else:
state["presence"] = content.pop("presence")
if "status_msg" in content: if "status_msg" in content:
state["status_msg"] = content.pop("status_msg") state["status_msg"] = content.pop("status_msg")

View file

@ -142,7 +142,7 @@ class PresenceStateTestCase(unittest.TestCase):
) )
self.assertEquals( self.assertEquals(
{"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, {"presence": ONLINE, "status_msg": "Online"},
state state
) )
mocked_get.assert_called_with("apple") mocked_get.assert_called_with("apple")
@ -159,7 +159,7 @@ class PresenceStateTestCase(unittest.TestCase):
) )
self.assertEquals( self.assertEquals(
{"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, {"presence": ONLINE, "status_msg": "Online"},
state state
) )
mocked_get.assert_called_with("apple") mocked_get.assert_called_with("apple")
@ -178,7 +178,7 @@ class PresenceStateTestCase(unittest.TestCase):
) )
self.assertEquals( self.assertEquals(
{"state": ONLINE, "presence": ONLINE, "status_msg": "Online"}, {"presence": ONLINE, "status_msg": "Online"},
state state
) )
@ -208,7 +208,8 @@ class PresenceStateTestCase(unittest.TestCase):
state={"presence": UNAVAILABLE, "status_msg": "Away"}) state={"presence": UNAVAILABLE, "status_msg": "Away"})
mocked_set.assert_called_with("apple", 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, self.mock_start.assert_called_with(self.u_apple,
state={ state={
"presence": UNAVAILABLE, "presence": UNAVAILABLE,
@ -460,8 +461,7 @@ class PresenceInvitesTestCase(unittest.TestCase):
self.assertEquals([ self.assertEquals([
{"observed_user": self.u_banana, {"observed_user": self.u_banana,
"presence": OFFLINE, "presence": OFFLINE},
"state": OFFLINE},
], presence) ], presence)
self.datastore.get_presence_list.assert_called_with("apple", self.datastore.get_presence_list.assert_called_with("apple",
@ -478,8 +478,7 @@ class PresenceInvitesTestCase(unittest.TestCase):
self.assertEquals([ self.assertEquals([
{"observed_user": self.u_banana, {"observed_user": self.u_banana,
"presence": OFFLINE, "presence": OFFLINE},
"state": OFFLINE},
], presence) ], presence)
self.datastore.get_presence_list.assert_called_with("apple", 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.room_members = [self.u_apple, self.u_elderberry]
self.datastore.set_presence_state.return_value = defer.succeed( self.datastore.set_presence_state.return_value = defer.succeed(
{"state": ONLINE}) {"state": ONLINE}
)
# TODO(paul): Gut-wrenching # TODO(paul): Gut-wrenching
self.handler._user_cachemap[self.u_apple] = UserPresenceCache() self.handler._user_cachemap[self.u_apple] = UserPresenceCache()
@ -654,7 +654,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@apple:test", "user_id": "@apple:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 0, "last_active_ago": 0,
}}, }},
], ],
@ -673,7 +672,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@apple:test", "user_id": "@apple:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 0, "last_active_ago": 0,
}}, }},
], ],
@ -692,7 +690,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@apple:test", "user_id": "@apple:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 0, "last_active_ago": 0,
}}, }},
], ],
@ -715,11 +712,9 @@ class PresencePushTestCase(unittest.TestCase):
self.assertEquals( self.assertEquals(
[ [
{"observed_user": self.u_banana, {"observed_user": self.u_banana,
"presence": OFFLINE, "presence": OFFLINE},
"state": OFFLINE},
{"observed_user": self.u_clementine, {"observed_user": self.u_clementine,
"presence": OFFLINE, "presence": OFFLINE},
"state": OFFLINE},
], ],
presence presence
) )
@ -740,11 +735,9 @@ class PresencePushTestCase(unittest.TestCase):
self.assertEquals([ self.assertEquals([
{"observed_user": self.u_banana, {"observed_user": self.u_banana,
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 2000}, "last_active_ago": 2000},
{"observed_user": self.u_clementine, {"observed_user": self.u_clementine,
"presence": OFFLINE, "presence": OFFLINE},
"state": OFFLINE},
], presence) ], presence)
(events, _) = yield self.event_source.get_new_events_for_user( (events, _) = yield self.event_source.get_new_events_for_user(
@ -758,7 +751,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@banana:test", "user_id": "@banana:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 2000 "last_active_ago": 2000
}}, }},
] ]
@ -775,7 +767,6 @@ class PresencePushTestCase(unittest.TestCase):
"push": [ "push": [
{"user_id": "@apple:test", {"user_id": "@apple:test",
"presence": u"online", "presence": u"online",
"state": u"online",
"last_active_ago": 0}, "last_active_ago": 0},
], ],
} }
@ -791,7 +782,6 @@ class PresencePushTestCase(unittest.TestCase):
"push": [ "push": [
{"user_id": "@apple:test", {"user_id": "@apple:test",
"presence": u"online", "presence": u"online",
"state": u"online",
"last_active_ago": 0}, "last_active_ago": 0},
], ],
} }
@ -837,7 +827,7 @@ class PresencePushTestCase(unittest.TestCase):
content={ content={
"push": [ "push": [
{"user_id": "@potato:remote", {"user_id": "@potato:remote",
"state": "online", "presence": "online",
"last_active_ago": 1000}, "last_active_ago": 1000},
], ],
} }
@ -855,7 +845,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@potato:remote", "user_id": "@potato:remote",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 1000, "last_active_ago": 1000,
}} }}
] ]
@ -866,7 +855,7 @@ class PresencePushTestCase(unittest.TestCase):
state = yield self.handler.get_state(self.u_potato, self.u_apple) state = yield self.handler.get_state(self.u_potato, self.u_apple)
self.assertEquals( self.assertEquals(
{"state": ONLINE, "presence": ONLINE, "last_active_ago": 3000}, {"presence": ONLINE, "last_active_ago": 3000},
state state
) )
@ -902,7 +891,6 @@ class PresencePushTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@clementine:test", "user_id": "@clementine:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 0, "last_active_ago": 0,
}} }}
] ]
@ -919,8 +907,7 @@ class PresencePushTestCase(unittest.TestCase):
content={ content={
"push": [ "push": [
{"user_id": "@apple:test", {"user_id": "@apple:test",
"presence": "online", "presence": "online"},
"state": "online"},
], ],
} }
), ),
@ -934,8 +921,7 @@ class PresencePushTestCase(unittest.TestCase):
content={ content={
"push": [ "push": [
{"user_id": "@banana:test", {"user_id": "@banana:test",
"presence": "offline", "presence": "offline"},
"state": "offline"},
], ],
} }
), ),
@ -964,8 +950,7 @@ class PresencePushTestCase(unittest.TestCase):
content={ content={
"push": [ "push": [
{"user_id": "@clementine:test", {"user_id": "@clementine:test",
"presence": "online", "presence": "online"},
"state": "online"},
], ],
} }
), ),
@ -1264,7 +1249,6 @@ class PresencePollingTestCase(unittest.TestCase):
"push": [ "push": [
{"user_id": "@banana:test", {"user_id": "@banana:test",
"presence": "offline", "presence": "offline",
"state": "offline",
"status_msg": None}, "status_msg": None},
], ],
}, },

View file

@ -148,10 +148,11 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
yield self.handlers.presence_handler.set_state( yield self.handlers.presence_handler.set_state(
target_user=self.u_apple, auth_user=self.u_apple, 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", mocked_set.assert_called_with("apple",
{"state": UNAVAILABLE, "status_msg": "Away"}) {"state": UNAVAILABLE, "status_msg": "Away"}
)
@defer.inlineCallbacks @defer.inlineCallbacks
def test_push_local(self): def test_push_local(self):
@ -161,7 +162,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
] ]
self.datastore.set_presence_state.return_value = defer.succeed( self.datastore.set_presence_state.return_value = defer.succeed(
{"state": ONLINE}) {"state": ONLINE}
)
# TODO(paul): Gut-wrenching # TODO(paul): Gut-wrenching
from synapse.handlers.presence import UserPresenceCache from synapse.handlers.presence import UserPresenceCache
@ -177,9 +179,11 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
apple_set.add(self.u_clementine) apple_set.add(self.u_clementine)
yield self.handlers.presence_handler.set_state(self.u_apple, 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, 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( presence = yield self.handlers.presence_handler.get_presence_list(
observer_user=self.u_apple, accepted=True) observer_user=self.u_apple, accepted=True)
@ -187,14 +191,12 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
self.assertEquals([ self.assertEquals([
{"observed_user": self.u_banana, {"observed_user": self.u_banana,
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"last_active_ago": 0, "last_active_ago": 0,
"displayname": "Frank", "displayname": "Frank",
"avatar_url": "http://foo"}, "avatar_url": "http://foo"},
{"observed_user": self.u_clementine, {"observed_user": self.u_clementine,
"presence": OFFLINE, "presence": OFFLINE}
"state": OFFLINE}], ], presence)
presence)
self.mock_update_client.assert_has_calls([ self.mock_update_client.assert_has_calls([
call(users_to_push=set([self.u_apple, self.u_banana, self.u_clementine]), 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( self.datastore.set_presence_state.return_value = defer.succeed(
{"state": ONLINE}) {"state": ONLINE}
)
# TODO(paul): Gut-wrenching # TODO(paul): Gut-wrenching
from synapse.handlers.presence import UserPresenceCache from synapse.handlers.presence import UserPresenceCache
@ -257,7 +260,8 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
apple_set.add(self.u_potato.domain) apple_set.add(self.u_potato.domain)
yield self.handlers.presence_handler.set_state(self.u_apple, 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( self.replication.send_edu.assert_called_with(
destination="remote", destination="remote",
@ -266,7 +270,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
"push": [ "push": [
{"user_id": "@apple:test", {"user_id": "@apple:test",
"presence": "online", "presence": "online",
"state": "online",
"last_active_ago": 0, "last_active_ago": 0,
"displayname": "Frank", "displayname": "Frank",
"avatar_url": "http://foo"}, "avatar_url": "http://foo"},
@ -283,18 +286,19 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
# TODO(paul): Gut-wrenching # TODO(paul): Gut-wrenching
potato_set = self.handlers.presence_handler._remote_recvmap.setdefault( potato_set = self.handlers.presence_handler._remote_recvmap.setdefault(
self.u_potato, set()) self.u_potato, set()
)
potato_set.add(self.u_apple) potato_set.add(self.u_apple)
yield self.replication.received_edu( yield self.replication.received_edu(
"remote", "m.presence", { "remote", "m.presence", {
"push": [ "push": [
{"user_id": "@potato:remote", {"user_id": "@potato:remote",
"state": "online", "presence": "online",
"displayname": "Frank", "displayname": "Frank",
"avatar_url": "http://foo"}, "avatar_url": "http://foo"},
], ],
} }
) )
self.mock_update_client.assert_called_with( self.mock_update_client.assert_called_with(
@ -313,7 +317,6 @@ class PresenceProfilelikeDataTestCase(unittest.TestCase):
self.assertEquals( self.assertEquals(
{"presence": ONLINE, {"presence": ONLINE,
"state": ONLINE,
"displayname": "Frank", "displayname": "Frank",
"avatar_url": "http://foo"}, "avatar_url": "http://foo"},
state) state)

View file

@ -99,7 +99,7 @@ class PresenceStateTestCase(unittest.TestCase):
self.assertEquals(200, code) self.assertEquals(200, code)
self.assertEquals( self.assertEquals(
{"presence": ONLINE, "state": ONLINE, "status_msg": "Available"}, {"presence": ONLINE, "status_msg": "Available"},
response response
) )
mocked_get.assert_called_with("apple") mocked_get.assert_called_with("apple")
@ -115,7 +115,8 @@ class PresenceStateTestCase(unittest.TestCase):
self.assertEquals(200, code) self.assertEquals(200, code)
mocked_set.assert_called_with("apple", mocked_set.assert_called_with("apple",
{"state": UNAVAILABLE, "status_msg": "Away"}) {"state": UNAVAILABLE, "status_msg": "Away"}
)
class PresenceListTestCase(unittest.TestCase): class PresenceListTestCase(unittest.TestCase):
@ -176,7 +177,7 @@ class PresenceListTestCase(unittest.TestCase):
self.assertEquals(200, code) self.assertEquals(200, code)
self.assertEquals([ self.assertEquals([
{"user_id": "@banana:test", "presence": OFFLINE, "state": OFFLINE}, {"user_id": "@banana:test", "presence": OFFLINE},
], response) ], response)
self.datastore.get_presence_list.assert_called_with( 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.room_members = [self.u_apple, self.u_banana]
self.mock_datastore.set_presence_state.return_value = defer.succeed( self.mock_datastore.set_presence_state.return_value = defer.succeed(
{"state": ONLINE}) {"state": ONLINE}
)
self.mock_datastore.get_presence_list.return_value = defer.succeed( self.mock_datastore.get_presence_list.return_value = defer.succeed(
[]) []
)
(code, response) = yield self.mock_resource.trigger("GET", (code, response) = yield self.mock_resource.trigger("GET",
"/events?timeout=0", None) "/events?timeout=0", None)
@ -329,9 +332,11 @@ class PresenceEventStreamTestCase(unittest.TestCase):
) )
self.mock_datastore.set_presence_state.return_value = defer.succeed( self.mock_datastore.set_presence_state.return_value = defer.succeed(
{"state": ONLINE}) {"state": ONLINE}
)
self.mock_datastore.get_presence_list.return_value = defer.succeed( self.mock_datastore.get_presence_list.return_value = defer.succeed(
[]) []
)
yield self.presence.set_state(self.u_banana, self.u_banana, yield self.presence.set_state(self.u_banana, self.u_banana,
state={"presence": ONLINE} state={"presence": ONLINE}
@ -346,7 +351,6 @@ class PresenceEventStreamTestCase(unittest.TestCase):
"content": { "content": {
"user_id": "@banana:test", "user_id": "@banana:test",
"presence": ONLINE, "presence": ONLINE,
"state": ONLINE,
"displayname": "Frank", "displayname": "Frank",
"last_active_ago": 0, "last_active_ago": 0,
}}, }},