Only send presence "poll"/"unpoll" EDUs when changing from/to zero remotes

2 files changed, 47 insertions, 8 deletions

diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py
index 1b3cdcc38c..bef1508892 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 13217d456b..8d094fd1f9 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):