diff options
3 files changed, 64 insertions, 56 deletions
diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py index a9fe58d9ff..5e3044d164 100644 --- a/synapse/server_notices/consent_server_notices.py +++ b/synapse/server_notices/consent_server_notices.py @@ -103,7 +103,7 @@ class ConsentServerNotices(object): }, ) yield self._server_notices_manager.send_notice( - user_id, content + user_id, content, ) yield self._store.user_set_consent_server_notice_sent( user_id, self._current_consent_version, diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py index 33625fba72..5987e448b2 100644 --- a/synapse/server_notices/resource_limits_server_notices.py +++ b/synapse/server_notices/resource_limits_server_notices.py @@ -14,40 +14,41 @@ # limitations under the License. import logging +from six import iteritems + from twisted.internet import defer from synapse.api.constants import EventTypes -from synapse.api.errors import AuthError, SynapseError +from synapse.api.errors import AuthError, ResourceLimitError, SynapseError from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG logger = logging.getLogger(__name__) class ResourceLimitsServerNotices(object): - """ + """ Keeps track of whether the server has reached it's resource limit and + ensures that the client is kept up to date. """ def __init__(self, hs): """ - Args: hs (synapse.server.HomeServer): """ self._server_notices_manager = hs.get_server_notices_manager() 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._auth = hs.get_auth() + self._config = hs.config self._resouce_limited = False self._message_handler = hs.get_message_handler() self._state = hs.get_state_handler() - # Config checks? @defer.inlineCallbacks def maybe_send_server_notice_to_user(self, user_id): - """Check if we need to send a notice to this user, and does so if so + """Check if we need to send a notice to this user, this will be true in + two cases. + 1. The server has reached its limit does not reflect this + 2. The room state indicates that the server has reached its limit when + actually the server is fine Args: user_id (str): user to check @@ -55,10 +56,10 @@ class ResourceLimitsServerNotices(object): Returns: Deferred """ - if self._hs_disabled is True: + if self._config.hs_disabled is True: return - if self._limit_usage_by_mau is False: + if self._config.limit_usage_by_mau is False: return timestamp = yield self._store.user_last_seen_monthly_active(user_id) @@ -78,8 +79,15 @@ class ResourceLimitsServerNotices(object): # 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() - if currently_blocked: + try: + yield self._auth.check_auth_blocking() + is_auth_blocking = False + except ResourceLimitError as e: + is_auth_blocking = True + event_content = e.msg + event_limit_type = e.limit_type + + if currently_blocked and not is_auth_blocking: # Room is notifying of a block, when it ought not to be. # Remove block notification content = { @@ -89,31 +97,29 @@ class ResourceLimitsServerNotices(object): user_id, content, EventTypes.Pinned, '', ) - except AuthError as e: + elif not currently_blocked and is_auth_blocking: + # Room is not notifying of a block, when it ought to be. + # Add block notification + content = { + 'body': event_content, + 'admin_uri': self._config.admin_uri, + 'limit_type': event_limit_type + } + event = yield self._server_notices_manager.send_notice( + user_id, content, EventTypes.ServerNoticeLimitReached + ) - try: - if not currently_blocked: - # Room is not notifying of a block, when it ought to be. - # Add block notification - content = { - 'body': e.msg, - 'admin_uri': self._admin_uri, - } - event = yield self._server_notices_manager.send_notice( - user_id, content, EventTypes.ServerNoticeLimitReached - ) - - content = { - "pinned": [ - event.event_id, - ] - } - yield self._server_notices_manager.send_notice( - user_id, content, EventTypes.Pinned, '', - ) - - except SynapseError as e: - logger.error("Error sending resource limits server notice: %s", e) + content = { + "pinned": [ + event.event_id, + ] + } + yield self._server_notices_manager.send_notice( + user_id, content, EventTypes.Pinned, '', + ) + + except SynapseError as e: + logger.error("Error sending resource limits server notice: %s", e) @defer.inlineCallbacks def _check_and_set_tags(self, user_id, room_id): @@ -167,7 +173,7 @@ class ResourceLimitsServerNotices(object): referenced_events = pinned_state_event.content.get('pinned') events = yield self._store.get_events(referenced_events) - for event_id, event in events.items(): + for event_id, event in events.iteritems(): if event.type == EventTypes.ServerNoticeLimitReached: currently_blocked = True # remove event in case we need to disable blocking later on. diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index ccb69097b1..cc8d2f539c 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -3,7 +3,7 @@ from mock import Mock from twisted.internet import defer from synapse.api.constants import EventTypes -from synapse.api.errors import AuthError +from synapse.api.errors import ResourceLimitError from synapse.handlers.auth import AuthHandler from synapse.server_notices.resource_limits_server_notices import ( ResourceLimitsServerNotices, @@ -43,13 +43,13 @@ class TestResourceLimitsServerNotices(unittest.TestCase): self._send_notice = self._rlsn._server_notices_manager.send_notice - self._rlsn._limit_usage_by_mau = True + self.hs.config.limit_usage_by_mau = True 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.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="" @@ -61,14 +61,14 @@ class TestResourceLimitsServerNotices(unittest.TestCase): def test_maybe_send_server_notice_to_user_flag_off(self): """Tests cases where the flags indicate nothing to do""" # test hs disabled case - self._hs_disabled = True + self.hs.config.hs_disabled = True 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 + self.hs.config.hs_disabled = False + self.hs.limit_usage_by_mau = False yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) self._send_notice.assert_not_called() @@ -77,7 +77,7 @@ 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.auth.check_auth_blocking = Mock() + 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} @@ -90,8 +90,8 @@ class TestResourceLimitsServerNotices(unittest.TestCase): @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.auth.check_auth_blocking = Mock( - side_effect=AuthError(403, 'foo') + self._rlsn._auth.check_auth_blocking = Mock( + side_effect=ResourceLimitError(403, 'foo') ) mock_event = Mock(type=EventTypes.ServerNoticeLimitReached) @@ -106,7 +106,9 @@ class TestResourceLimitsServerNotices(unittest.TestCase): 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')) + self._rlsn._auth.check_auth_blocking = Mock( + side_effect=ResourceLimitError(403, 'foo') + ) yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) # Would be better to check contents, but 2 calls == set blocking event @@ -116,7 +118,7 @@ class TestResourceLimitsServerNotices(unittest.TestCase): def test_maybe_send_server_notice_to_user_add_blocked_notice_noop(self): """Test when user does not have blocked notice, nor should they (NOOP)""" - self._rlsn.auth.check_auth_blocking = Mock() + self._rlsn._auth.check_auth_blocking = Mock() yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) @@ -129,7 +131,7 @@ class TestResourceLimitsServerNotices(unittest.TestCase): happen - but ... """ - self._rlsn.auth.check_auth_blocking = Mock() + self._rlsn._auth.check_auth_blocking = Mock() self._rlsn._store.user_last_seen_monthly_active = Mock( return_value=defer.succeed(None) ) |