From 93407cf7cff05fa99709ba5055bce329524cef32 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 29 Aug 2014 19:53:33 +0100 Subject: [PATCH] Bugfixes on presence pushes on user joining: * No need to inform clients of status of remote users; as that will arrive in due course anyway. We don't -have- the state currently, so we'd only send an unknown message * Remember to bump the presence serial for the event source, so the notifiers will wake up and report it --- synapse/handlers/presence.py | 17 ++--- tests/handlers/test_presence.py | 128 ++++++++++++++++++-------------- 2 files changed, 79 insertions(+), 66 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index cc28151e35..93bd07b196 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -260,19 +260,18 @@ class PresenceHandler(BaseHandler): @defer.inlineCallbacks def user_joined_room(self, user, room_id): - if user.is_mine: + statuscache = self._get_or_make_usercache(user) + + # No actual update but we need to bump the serial anyway for the + # event source + self._user_cachemap_latest_serial += 1 + statuscache.update({}, serial=self._user_cachemap_latest_serial) + self.push_update_to_local_and_remote( observed_user=user, room_ids=[room_id], - statuscache=self._get_or_offline_usercache(user), - ) - - else: - self.push_update_to_clients( - observed_user=user, - room_ids=[room_id], - statuscache=self._get_or_offline_usercache(user), + statuscache=statuscache, ) # We also want to tell them about current presence of people. diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index fcd7a784cd..0a176bdd44 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -514,13 +514,6 @@ class PresencePushTestCase(unittest.TestCase): ) hs.handlers = JustPresenceHandlers(hs) - def update(*args,**kwargs): - # print "mock_update_client: Args=%s, kwargs=%s" %(args, kwargs,) - return defer.succeed(None) - - self.mock_update_client = Mock() - self.mock_update_client.side_effect = update - self.datastore = hs.get_datastore() def get_received_txn_response(*args): @@ -528,7 +521,7 @@ class PresencePushTestCase(unittest.TestCase): self.datastore.get_received_txn_response = get_received_txn_response self.handler = hs.get_handlers().presence_handler - self.handler.push_update_to_clients = self.mock_update_client + self.event_source = hs.get_event_sources().sources["presence"] # Mock the RoomMemberHandler hs.handlers.room_member_handler = Mock(spec=[ @@ -622,16 +615,23 @@ class PresencePushTestCase(unittest.TestCase): apple_set.add(self.u_banana) apple_set.add(self.u_clementine) + self.assertEquals(self.event_source.get_current_key(), 0) + yield self.handler.set_state(self.u_apple, self.u_apple, {"state": ONLINE}) - self.mock_update_client.assert_has_calls([ - call(users_to_push=set([self.u_apple, self.u_banana, self.u_clementine]), - room_ids=["a-room"], - observed_user=self.u_apple, - statuscache=ANY), # self-reflection - ], any_order=True) - self.mock_update_client.reset_mock() + self.assertEquals(self.event_source.get_current_key(), 1) + self.assertEquals( + self.event_source.get_new_events_for_user(self.u_apple, 0, None)[0], + [ + {"type": "m.presence", + "content": { + "user_id": "@apple:test", + "state": ONLINE, + "mtime_age": 0, + }}, + ], + ) presence = yield self.handler.get_presence_list( observer_user=self.u_apple, accepted=True) @@ -657,31 +657,24 @@ class PresencePushTestCase(unittest.TestCase): "state": OFFLINE}, ], presence) - self.mock_update_client.assert_has_calls([ - call(users_to_push=set([self.u_banana]), - room_ids=[], - observed_user=self.u_banana, - statuscache=ANY), # self-reflection - ]) # and no others... + self.assertEquals(self.event_source.get_current_key(), 2) + self.assertEquals( + self.event_source.get_new_events_for_user( + self.u_banana, 1, None + )[0], + [ + {"type": "m.presence", + "content": { + "user_id": "@banana:test", + "state": ONLINE, + "mtime_age": 2000 + }}, + ] + ) @defer.inlineCallbacks def test_push_remote(self): put_json = self.mock_http_client.put_json -# put_json.expect_call_and_return( -# call("remote", -# path=ANY, # Can't guarantee which txn ID will be which -# data=_expect_edu("remote", "m.presence", -# content={ -# "push": [ -# {"user_id": "@apple:test", -# "state": "online", -# "mtime_age": 0}, -# ], -# } -# ) -# ), -# defer.succeed((200, "OK")) -# ) put_json.expect_call_and_return( call("farm", path=ANY, # Can't guarantee which txn ID will be which @@ -724,6 +717,8 @@ class PresencePushTestCase(unittest.TestCase): self.room_members = [self.u_banana, self.u_potato] + self.assertEquals(self.event_source.get_current_key(), 0) + yield self.mock_federation_resource.trigger("PUT", "/matrix/federation/v1/send/1000000/", _make_edu_json("elsewhere", "m.presence", @@ -737,12 +732,20 @@ class PresencePushTestCase(unittest.TestCase): ) ) - self.mock_update_client.assert_has_calls([ - call(users_to_push=set([self.u_apple]), - room_ids=["a-room"], - observed_user=self.u_potato, - statuscache=ANY), - ], any_order=True) + self.assertEquals(self.event_source.get_current_key(), 1) + self.assertEquals( + self.event_source.get_new_events_for_user( + self.u_apple, 0, None + )[0], + [ + {"type": "m.presence", + "content": { + "user_id": "@potato:remote", + "state": ONLINE, + "mtime_age": 1000, + }} + ] + ) self.clock.advance_time(2) @@ -754,24 +757,35 @@ class PresencePushTestCase(unittest.TestCase): def test_join_room_local(self): self.room_members = [self.u_apple, self.u_banana] - yield self.distributor.fire("user_joined_room", self.u_elderberry, + self.assertEquals(self.event_source.get_current_key(), 0) + + # TODO(paul): Gut-wrenching + self.handler._user_cachemap[self.u_clementine] = UserPresenceCache() + self.handler._user_cachemap[self.u_clementine].update( + { + "state": PresenceState.ONLINE, + "mtime": self.clock.time_msec(), + }, self.u_clementine + ) + + yield self.distributor.fire("user_joined_room", self.u_clementine, "a-room" ) - self.mock_update_client.assert_has_calls([ - call(room_ids=["a-room"], - observed_user=self.u_elderberry, - users_to_push=set(), - statuscache=ANY), - call(users_to_push=set([self.u_elderberry]), - observed_user=self.u_apple, - room_ids=[], - statuscache=ANY), - call(users_to_push=set([self.u_elderberry]), - observed_user=self.u_banana, - room_ids=[], - statuscache=ANY), - ], any_order=True) + self.assertEquals(self.event_source.get_current_key(), 1) + self.assertEquals( + self.event_source.get_new_events_for_user( + self.u_apple, 0, None + )[0], + [ + {"type": "m.presence", + "content": { + "user_id": "@clementine:test", + "state": ONLINE, + "mtime_age": 0, + }} + ] + ) @defer.inlineCallbacks def test_join_room_remote(self):