check for room state before deciding on action

3 files changed, 58 insertions, 87 deletions

diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py
index 783ceae1ad..a9fe58d9ff 100644
--- a/synapse/server_notices/consent_server_notices.py
+++ b/synapse/server_notices/consent_server_notices.py
@@ -22,7 +22,6 @@ from synapse.api.errors import SynapseError
 from synapse.api.urls import ConsentURIBuilder
 from synapse.config import ConfigError
 from synapse.types import get_localpart_from_id
-from synapse.api.constants import EventTypes
 
 logger = logging.getLogger(__name__)
 
diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 2b714a1016..43a926fc79 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -16,8 +16,8 @@ import logging
 
 from twisted.internet import defer
 
-from synapse.api.errors import AuthError, SynapseError
 from synapse.api.constants import EventTypes
+from synapse.api.errors import AuthError, SynapseError
 
 logger = logging.getLogger(__name__)
 
@@ -35,10 +35,10 @@ class ResourceLimitsServerNotices(object):
         self._store = hs.get_datastore()
         self.auth = hs.get_auth()
         self._server_notice_content = hs.config.user_consent_server_notice_content
+        self._admin_uri = hs.config.admin_uri
         self._limit_usage_by_mau = hs.config.limit_usage_by_mau
         self._hs_disabled = hs.config.hs_disabled
 
-        self._notified_of_blocking = set()
         self._resouce_limited = False
         self._message_handler = hs.get_message_handler()
         self._state = hs.get_state_handler()
@@ -58,83 +58,45 @@ class ResourceLimitsServerNotices(object):
             return
 
         if self._limit_usage_by_mau is True:
-            room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
+            timestamp = yield self._store.user_last_seen_monthly_active(user_id)
+            if timestamp is None:
+                # This user will be blocked from receiving the notice anyway.
+                # In practice, not sure we can ever get here
+                return
 
+            room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
 
-            # Alternate impl - currently inlcuded because I'm not sure I am on
-            # the right track and want to share WIP
-
-            # logger.info("GET STATE EVENTS")
-            # currently_blocked = False
-            # events = []
-            # try:
-            #     events = yield self._message_handler.get_state_events(user_id, room_id, types=[(EventTypes.Pinned, None)])
-            # except AuthError as e:
-            #     # The user has yet to join the server notices room
-            #     pass
-            #
-            # pinned_event_refs = []
-            # for e in events:
-            #     logger.info('events %s' % e)
-            #     logger.info(type(e))
-            #     for key, event_ids in e['content'].items():
-            #         logger.info('Key Event %s %s' % (key, event_ids))
-            #         if key == 'pinned':
-            #             pinned_event_refs = event_ids
-            #
-            # logger.info('pinned_event_refs %s' % pinned_event_refs)
-            #
-            # events = yield self._store.get_events(pinned_event_refs)
-            # logger.info(events)
-            # for event_id, event in events.items():
-            #     logger.info("event_id, event event.type %s %s %s" % (event_id, event, event.type))
-            #     if event.type == 'm.server_notice.usage_limit_reached':
-            #         currently_blocked = True
-            #
-            # logger.info('Currently Blocked is %r' % currently_blocked)
-
-            #for e in events:
-            #    logger.info(e)
             currently_blocked = False
             logger.info("GET CURRENT STATE")
-            pinned_state_event = yield self._state.get_current_state(room_id, event_type=EventTypes.Pinned)
-            logger.info(events)
-            logger.info(events.get('content'))
+            pinned_state_event = None
+            try:
+                pinned_state_event = yield self._state.get_current_state(
+                    room_id, event_type=EventTypes.Pinned
+                )
+            except AuthError as e:
+                # The user has yet to join the server notices room
+                pass
 
             referenced_events = []
             if pinned_state_event is not None:
-                content = pinned_state_event.get('content')
-                if content is not None:
-                    referenced_events = content.get('pinned')
+                referenced_events = pinned_state_event.content.get('pinned')
 
             events = yield self._store.get_events(referenced_events)
             logger.info(events)
             for event_id, event in events.items():
-                logger.info("event_id, event event.type %s %s %s" % (event_id, event, event.type))
-                if event.type == 'm.server_notice.usage_limit_reached':
+                logger.info("event_id, event event.type %s %s %s" % (
+                    event_id, event, event.type)
+                )
+                if event.type == EventTypes.ServerNoticeLimitReached:
                     currently_blocked = True
 
             logger.info("currently_blocked is %r" % currently_blocked)
-
-                #event = yield self._store.get_event(events.event_id)
-                #logger.info(event)
-
-            #logger.info("GET CURRENT STATE IDs")
-            #events = yield self._state.get_current_state_ids(room_id)
-            #for k,v in events.items():
-            #    logger.info('%s %s' % (k,v))
-
-            timestamp = yield self._store.user_last_seen_monthly_active(user_id)
-            if timestamp is None:
-                # This user will be blocked from receiving the notice anyway.
-                # In practice, not sure we can ever get here
-                return
             try:
                 # Normally should always pass in user_id if you have it, but in
                 # this case are checking what would happen to other users if they
                 # were to arrive.
                 yield self.auth.check_auth_blocking()
-                self._resouce_limited = False
+
                 # Need to start removing notices
                 # if user_id in self._notified_of_blocking:
                 if currently_blocked:
@@ -142,7 +104,7 @@ class ResourceLimitsServerNotices(object):
                     # send state event here
                     # How do I do this? if drop the id, how to refer to it?
                     content = {
-                        "pinned":[]
+                        "pinned": []
                     }
                     yield self._server_notices_manager.send_notice(
                         user_id, content, EventTypes.Pinned, '',
@@ -152,13 +114,11 @@ class ResourceLimitsServerNotices(object):
             except AuthError as e:
                 # Need to start notifying of blocking
                 try:
-                    self._resouce_limited = True
-                    #if user_id not in self._notified_of_blocking:
                     if not currently_blocked:
                         # TODO use admin email contained in error once PR lands
                         content = {
                             'body': e.msg,
-                            'admin_email': 'stunt@adminemail.com',
+                            'admin_uri': self._admin_uri,
                         }
                         event = yield self._server_notices_manager.send_notice(
                             user_id, content, EventTypes.ServerNoticeLimitReached
@@ -167,7 +127,7 @@ class ResourceLimitsServerNotices(object):
                         # send server notices state event here
                         # TODO Over writing pinned events
                         content = {
-                            "pinned":[
+                            "pinned": [
                                 event.event_id,
                             ]
                         }
@@ -177,4 +137,4 @@ class ResourceLimitsServerNotices(object):
                         )
 
                 except SynapseError as e:
-                    logger.error("Error sending server notice about resource limits: %s", e)
+                    logger.error("Error sending resource limits server notice: %s", e)
diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py
index a69253f1b6..fb8c593a75 100644
--- a/tests/server_notices/test_resource_limits_server_notices.py
+++ b/tests/server_notices/test_resource_limits_server_notices.py
@@ -2,6 +2,7 @@ from mock import Mock
 
 from twisted.internet import defer
 
+from synapse.api.constants import EventTypes
 from synapse.api.errors import AuthError
 from synapse.handlers.auth import AuthHandler
 from synapse.server_notices.resource_limits_server_notices import (
@@ -20,7 +21,7 @@ class AuthHandlers(object):
 class TestResourceLimitsServerNotices(unittest.TestCase):
     @defer.inlineCallbacks
     def setUp(self):
-        self.hs = yield setup_test_homeserver(handlers=None)
+        self.hs = yield setup_test_homeserver(self.addCleanup, handlers=None)
         self.hs.handlers = AuthHandlers(self.hs)
         self.auth_handler = self.hs.handlers.auth_handler
         self.server_notices_sender = self.hs.get_server_notices_sender()
@@ -37,10 +38,23 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         )
         self._send_notice = self._rlsn._server_notices_manager.send_notice
         self._rlsn._server_notices_manager.send_notice = Mock()
+        self._rlsn._state.get_current_state = Mock(return_value=defer.succeed(None))
+        self._rlsn._store.get_events = Mock(return_value=defer.succeed({}))
+
         self._send_notice = self._rlsn._server_notices_manager.send_notice
 
         self._rlsn._limit_usage_by_mau = True
-        self.user_id = "user_id"
+        self.user_id = "@user_id:test"
+
+        self.server_notices_mxid = "@server:test"
+        self.server_notices_mxid_display_name = None
+        self.server_notices_mxid_avatar_url = None
+        self.server_notices_room_name = "Server Notices"
+
+        self._rlsn._server_notices_manager.get_notice_room_for_user = Mock(
+            returnValue=""
+        )
+        self.hs.config.admin_uri = "mailto:user@test.com"
 
     @defer.inlineCallbacks
     def test_maybe_send_server_notice_to_user_flag_off(self):
@@ -48,13 +62,13 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         # test hs disabled case
         self._hs_disabled = True
 
-        yield self._rlsn.maybe_send_server_notice_to_user("user_id")
+        yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
         self._send_notice.assert_not_called()
         # Test when mau limiting disabled
         self._hs_disabled = False
         self._rlsn._limit_usage_by_mau = False
-        yield self._rlsn.maybe_send_server_notice_to_user("user_id")
+        yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
         self._send_notice.assert_not_called()
 
@@ -62,40 +76,40 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
     def test_maybe_send_server_notice_to_user_remove_blocked_notice(self):
         """Test when user has blocked notice, but should have it removed"""
 
-        self._rlsn._notified_of_blocking.add(self.user_id)
         self._rlsn.auth.check_auth_blocking = Mock()
+        mock_event = Mock(type=EventTypes.ServerNoticeLimitReached)
+        self._rlsn._store.get_events = Mock(return_value=defer.succeed(
+            {"123": mock_event}
+        ))
 
         yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
-        # "remove warning" obviously aweful, but test will start failing when code
-        # actually sends a real event, and then it can be updated
-
-        self._send_notice.assert_called_once_with(self.user_id, "remove warning")
-        self.assertFalse(self.user_id in self._rlsn._notified_of_blocking)
+        # Would be better to check the content, but once == remove blocking event
+        self._send_notice.assert_called_once()
 
     @defer.inlineCallbacks
     def test_maybe_send_server_notice_to_user_remove_blocked_notice_noop(self):
         """Test when user has blocked notice, but notice ought to be there (NOOP)"""
-        self._rlsn._notified_of_blocking.add(self.user_id)
         self._rlsn.auth.check_auth_blocking = Mock(
             side_effect=AuthError(403, 'foo')
         )
 
-        yield self._rlsn.maybe_send_server_notice_to_user("user_id")
+        mock_event = Mock(type=EventTypes.ServerNoticeLimitReached)
+        self._rlsn._store.get_events = Mock(return_value=defer.succeed(
+            {"123": mock_event}
+        ))
+        yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
         self._send_notice.assert_not_called()
-        self.assertTrue(self.user_id in self._rlsn._notified_of_blocking)
 
     @defer.inlineCallbacks
     def test_maybe_send_server_notice_to_user_add_blocked_notice(self):
         """Test when user does not have blocked notice, but should have one"""
 
         self._rlsn.auth.check_auth_blocking = Mock(side_effect=AuthError(403, 'foo'))
-        yield self._rlsn.maybe_send_server_notice_to_user("user_id")
+        yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
-        # "add warning" obviously awful, but test will start failing when code
-        # actually sends a real event, and then it can be updated
-        self._send_notice.assert_called_once_with(self.user_id, "add warning")
-        self.assertTrue(self.user_id in self._rlsn._notified_of_blocking)
+        # Would be better to check contents, but 2 calls == set blocking event
+        self.assertTrue(self._send_notice.call_count == 2)
 
     @defer.inlineCallbacks
     def test_maybe_send_server_notice_to_user_add_blocked_notice_noop(self):
@@ -106,7 +120,6 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
         self._send_notice.assert_not_called()
-        self.assertFalse(self.user_id in self._rlsn._notified_of_blocking)
 
     @defer.inlineCallbacks
     def test_maybe_send_server_notice_to_user_not_in_mau_cohort(self):
@@ -122,4 +135,3 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
 
         self._send_notice.assert_not_called()
-        self.assertFalse(self.user_id in self._rlsn._notified_of_blocking)