diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 1b3cdcc38..bef150889 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -437,16 +437,22 @@ class PresenceHandler(BaseHandler): ) def _start_polling_remote(self, user, domain, remoteusers): + to_poll = set() + for u in remoteusers: if u not in self._remote_recvmap: self._remote_recvmap[u] = set() + to_poll.add(u) self._remote_recvmap[u].add(user) + if not to_poll: + return defer.succeed(None) + return self.federation.send_edu( destination=domain, edu_type="m.presence", - content={"poll": [u.to_string() for u in remoteusers]} + content={"poll": [u.to_string() for u in to_poll]} ) def stop_polling_presence(self, user, target_user=None): @@ -489,16 +495,22 @@ class PresenceHandler(BaseHandler): del self._local_pushmap[localpart] def _stop_polling_remote(self, user, domain, remoteusers): + to_unpoll = set() + for u in remoteusers: self._remote_recvmap[u].remove(user) if not self._remote_recvmap[u]: del self._remote_recvmap[u] + to_unpoll.add(u) + + if not to_unpoll: + return defer.succeed(None) return self.federation.send_edu( destination=domain, edu_type="m.presence", - content={"unpoll": [u.to_string() for u in remoteusers]} + content={"unpoll": [u.to_string() for u in to_unpoll]} ) @defer.inlineCallbacks diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 13217d456..8d094fd1f 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -15,7 +15,7 @@ from twisted.trial import unittest -from twisted.internet import defer +from twisted.internet import defer, reactor from mock import Mock, call, ANY import logging @@ -853,6 +853,7 @@ class PresencePollingTestCase(unittest.TestCase): 'apple': [ "@banana:test", "@clementine:test" ], 'banana': [ "@apple:test" ], 'clementine': [ "@apple:test", "@potato:remote" ], + 'fig': [ "@potato:remote" ], } @@ -902,9 +903,10 @@ class PresencePollingTestCase(unittest.TestCase): # Mocked database state # Local users always start offline self.current_user_state = { - "apple": OFFLINE, - "banana": OFFLINE, - "clementine": OFFLINE, + "apple": OFFLINE, + "banana": OFFLINE, + "clementine": OFFLINE, + "fig": OFFLINE, } def get_presence_state(user_localpart): @@ -934,6 +936,7 @@ class PresencePollingTestCase(unittest.TestCase): self.u_apple = hs.parse_userid("@apple:test") self.u_banana = hs.parse_userid("@banana:test") self.u_clementine = hs.parse_userid("@clementine:test") + self.u_fig = hs.parse_userid("@fig:test") # Remote users self.u_potato = hs.parse_userid("@potato:remote") @@ -1023,10 +1026,32 @@ class PresencePollingTestCase(unittest.TestCase): yield put_json.await_calls() # Gut-wrenching tests - self.assertTrue(self.u_potato in self.handler._remote_recvmap) + self.assertTrue(self.u_potato in self.handler._remote_recvmap, + msg="expected potato to be in _remote_recvmap" + ) self.assertTrue(self.u_clementine in self.handler._remote_recvmap[self.u_potato]) + # 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} + ) + + reactor.iterate(delay=0) + + put_json.assert_had_no_calls() + + # fig goes offline + yield self.handler.set_state( + target_user=self.u_fig, auth_user=self.u_fig, + state={"state": OFFLINE} + ) + + reactor.iterate(delay=0) + + put_json.assert_had_no_calls() + put_json.expect_call_and_return( call("remote", path="/matrix/federation/v1/send/1000001/", @@ -1046,7 +1071,9 @@ class PresencePollingTestCase(unittest.TestCase): put_json.await_calls() - self.assertFalse(self.u_potato in self.handler._remote_recvmap) + self.assertFalse(self.u_potato in self.handler._remote_recvmap, + msg="expected potato not to be in _remote_recvmap" + ) @defer.inlineCallbacks def test_remote_poll_receive(self):