diff options
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) |