clean up, no functional changes

This commit is contained in:
Neil Johnson 2018-08-17 15:21:34 +01:00
parent 3ee57bdcbb
commit d49b77404b
3 changed files with 63 additions and 55 deletions

View File

@ -103,7 +103,7 @@ class ConsentServerNotices(object):
}, },
) )
yield self._server_notices_manager.send_notice( yield self._server_notices_manager.send_notice(
user_id, content user_id, content,
) )
yield self._store.user_set_consent_server_notice_sent( yield self._store.user_set_consent_server_notice_sent(
user_id, self._current_consent_version, user_id, self._current_consent_version,

View File

@ -14,40 +14,41 @@
# limitations under the License. # limitations under the License.
import logging import logging
from six import iteritems
from twisted.internet import defer from twisted.internet import defer
from synapse.api.constants import EventTypes 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 from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class ResourceLimitsServerNotices(object): 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): def __init__(self, hs):
""" """
Args: Args:
hs (synapse.server.HomeServer): hs (synapse.server.HomeServer):
""" """
self._server_notices_manager = hs.get_server_notices_manager() self._server_notices_manager = hs.get_server_notices_manager()
self._store = hs.get_datastore() self._store = hs.get_datastore()
self.auth = hs.get_auth() self._auth = hs.get_auth()
self._server_notice_content = hs.config.user_consent_server_notice_content self._config = hs.config
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._resouce_limited = False self._resouce_limited = False
self._message_handler = hs.get_message_handler() self._message_handler = hs.get_message_handler()
self._state = hs.get_state_handler() self._state = hs.get_state_handler()
# Config checks?
@defer.inlineCallbacks @defer.inlineCallbacks
def maybe_send_server_notice_to_user(self, user_id): 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: Args:
user_id (str): user to check user_id (str): user to check
@ -55,10 +56,10 @@ class ResourceLimitsServerNotices(object):
Returns: Returns:
Deferred Deferred
""" """
if self._hs_disabled is True: if self._config.hs_disabled is True:
return return
if self._limit_usage_by_mau is False: if self._config.limit_usage_by_mau is False:
return return
timestamp = yield self._store.user_last_seen_monthly_active(user_id) 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 # 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 # this case are checking what would happen to other users if they
# were to arrive. # were to arrive.
yield self.auth.check_auth_blocking() try:
if currently_blocked: 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. # Room is notifying of a block, when it ought not to be.
# Remove block notification # Remove block notification
content = { content = {
@ -89,31 +97,29 @@ class ResourceLimitsServerNotices(object):
user_id, content, EventTypes.Pinned, '', 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: content = {
if not currently_blocked: "pinned": [
# Room is not notifying of a block, when it ought to be. event.event_id,
# Add block notification ]
content = { }
'body': e.msg, yield self._server_notices_manager.send_notice(
'admin_uri': self._admin_uri, user_id, content, EventTypes.Pinned, '',
} )
event = yield self._server_notices_manager.send_notice(
user_id, content, EventTypes.ServerNoticeLimitReached
)
content = { except SynapseError as e:
"pinned": [ logger.error("Error sending resource limits server notice: %s", e)
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 @defer.inlineCallbacks
def _check_and_set_tags(self, user_id, room_id): 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') referenced_events = pinned_state_event.content.get('pinned')
events = yield self._store.get_events(referenced_events) 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: if event.type == EventTypes.ServerNoticeLimitReached:
currently_blocked = True currently_blocked = True
# remove event in case we need to disable blocking later on. # remove event in case we need to disable blocking later on.

View File

@ -3,7 +3,7 @@ from mock import Mock
from twisted.internet import defer from twisted.internet import defer
from synapse.api.constants import EventTypes 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.handlers.auth import AuthHandler
from synapse.server_notices.resource_limits_server_notices import ( from synapse.server_notices.resource_limits_server_notices import (
ResourceLimitsServerNotices, ResourceLimitsServerNotices,
@ -43,13 +43,13 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
self._send_notice = self._rlsn._server_notices_manager.send_notice 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.user_id = "@user_id:test"
self.server_notices_mxid = "@server:test" # self.server_notices_mxid = "@server:test"
self.server_notices_mxid_display_name = None # self.server_notices_mxid_display_name = None
self.server_notices_mxid_avatar_url = None # self.server_notices_mxid_avatar_url = None
self.server_notices_room_name = "Server Notices" # self.server_notices_room_name = "Server Notices"
self._rlsn._server_notices_manager.get_notice_room_for_user = Mock( self._rlsn._server_notices_manager.get_notice_room_for_user = Mock(
returnValue="" returnValue=""
@ -61,14 +61,14 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
def test_maybe_send_server_notice_to_user_flag_off(self): def test_maybe_send_server_notice_to_user_flag_off(self):
"""Tests cases where the flags indicate nothing to do""" """Tests cases where the flags indicate nothing to do"""
# test hs disabled case # 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) yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
self._send_notice.assert_not_called() self._send_notice.assert_not_called()
# Test when mau limiting disabled # Test when mau limiting disabled
self._hs_disabled = False self.hs.config.hs_disabled = False
self._rlsn._limit_usage_by_mau = False self.hs.limit_usage_by_mau = False
yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
self._send_notice.assert_not_called() 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): def test_maybe_send_server_notice_to_user_remove_blocked_notice(self):
"""Test when user has blocked notice, but should have it removed""" """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) mock_event = Mock(type=EventTypes.ServerNoticeLimitReached)
self._rlsn._store.get_events = Mock(return_value=defer.succeed( self._rlsn._store.get_events = Mock(return_value=defer.succeed(
{"123": mock_event} {"123": mock_event}
@ -90,8 +90,8 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
@defer.inlineCallbacks @defer.inlineCallbacks
def test_maybe_send_server_notice_to_user_remove_blocked_notice_noop(self): 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)""" """Test when user has blocked notice, but notice ought to be there (NOOP)"""
self._rlsn.auth.check_auth_blocking = Mock( self._rlsn._auth.check_auth_blocking = Mock(
side_effect=AuthError(403, 'foo') side_effect=ResourceLimitError(403, 'foo')
) )
mock_event = Mock(type=EventTypes.ServerNoticeLimitReached) 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): def test_maybe_send_server_notice_to_user_add_blocked_notice(self):
"""Test when user does not have blocked notice, but should have one""" """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) yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
# Would be better to check contents, but 2 calls == set blocking event # 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): 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)""" """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) yield self._rlsn.maybe_send_server_notice_to_user(self.user_id)
@ -129,7 +131,7 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
happen - but ... 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( self._rlsn._store.user_last_seen_monthly_active = Mock(
return_value=defer.succeed(None) return_value=defer.succeed(None)
) )