diff options
3 files changed, 190 insertions, 45 deletions
diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py index 33b1d80cd9..017828f81e 100644 --- a/synapse/server_notices/resource_limits_server_notices.py +++ b/synapse/server_notices/resource_limits_server_notices.py @@ -14,14 +14,9 @@ # limitations under the License. import logging -from six import iteritems, string_types - from twisted.internet import defer -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.errors import AuthError, SynapseError logger = logging.getLogger(__name__) @@ -37,12 +32,12 @@ class ResourceLimitsServerNotices(object): """ self._server_notices_manager = hs.get_server_notices_manager() self._store = hs.get_datastore() - self._api = hs.get_api() + self.auth = hs.get_auth() self._server_notice_content = hs.config.user_consent_server_notice_content - self._limit_usage_by_mau = config.limit_usage_by_mau = False - self._hs_disabled.config.hs_disabled = False + self._limit_usage_by_mau = hs.config.limit_usage_by_mau = False + self._hs_disabled = hs.config.hs_disabled - self._notified = set() + self._notified_of_blocking = set() self._resouce_limited = False # Config checks? @@ -56,29 +51,49 @@ class ResourceLimitsServerNotices(object): Returns: Deferred """ - if self._limit_usage_by_mau is False and self._hs_disabled is False: - # not enabled + if self._hs_disabled is True: return - 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 - return - try: - yield self.api.check_auth_blocking() - if self._resouce_limited: + if self._limit_usage_by_mau is True: + 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: + yield self.auth.check_auth_blocking() + self._resouce_limited = False # Need to start removing notices - pass - except AuthError as e: - # Need to start notifying of blocking - if not self._resouce_limited: - pass + if user_id in self._notified_of_blocking: + # Send message to remove warning - needs updating + content = "remove warning" + self._send_server_notice(user_id, content) + self._notified_of_blocking.remove(user_id) - # need to send a message. - try: - yield self._server_notices_manager.send_notice( - user_id, content, - ) + except AuthError: + # Need to start notifying of blocking + + self._resouce_limited = True + if user_id not in self._notified_of_blocking: + # Send message to add warning - needs updating + content = "add warning" + self._send_server_notice(user_id, content) + self._notified_of_blocking.add(user_id) - except SynapseError as e: - logger.error("Error sending server notice about resource limits: %s", e) + @defer.inlineCallbacks + def _send_server_notice(self, user_id, content): + """Sends Server notice + + Args: + user_id(str): The user to send to + content(str): The content of the message + + Returns: + Deferred[] + """ + try: + yield self._server_notices_manager.send_notice( + user_id, content, + ) + except SynapseError as e: + logger.error("Error sending server notice about resource limits: %s", e) diff --git a/synapse/server_notices/server_notices_sender.py b/synapse/server_notices/server_notices_sender.py index 5d23965f34..6121b2f267 100644 --- a/synapse/server_notices/server_notices_sender.py +++ b/synapse/server_notices/server_notices_sender.py @@ -12,7 +12,12 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from twisted.internet import defer + from synapse.server_notices.consent_server_notices import ConsentServerNotices +from synapse.server_notices.resource_limits_server_notices import ( + ResourceLimitsServerNotices, +) class ServerNoticesSender(object): @@ -25,34 +30,34 @@ class ServerNoticesSender(object): Args: hs (synapse.server.HomeServer): """ - # todo: it would be nice to make this more dynamic - self._consent_server_notices = ConsentServerNotices(hs) + self._server_notices = ( + ConsentServerNotices(hs), + ResourceLimitsServerNotices(hs) + ) + @defer.inlineCallbacks def on_user_syncing(self, user_id): """Called when the user performs a sync operation. Args: user_id (str): mxid of user who synced - - Returns: - Deferred """ - return self._consent_server_notices.maybe_send_server_notice_to_user( - user_id, - ) + for sn in self._server_notices: + yield sn.maybe_send_server_notice_to_user( + user_id, + ) + @defer.inlineCallbacks def on_user_ip(self, user_id): """Called on the master when a worker process saw a client request. Args: user_id (str): mxid - - Returns: - Deferred """ # The synchrotrons use a stubbed version of ServerNoticesSender, so # we check for notices to send to the user in on_user_ip as well as # in on_user_syncing - return self._consent_server_notices.maybe_send_server_notice_to_user( - user_id, - ) + for sn in self._server_notices: + yield sn.maybe_send_server_notice_to_user( + user_id, + ) diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py new file mode 100644 index 0000000000..a69253f1b6 --- /dev/null +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -0,0 +1,125 @@ +from mock import Mock + +from twisted.internet import defer + +from synapse.api.errors import AuthError +from synapse.handlers.auth import AuthHandler +from synapse.server_notices.resource_limits_server_notices import ( + ResourceLimitsServerNotices, +) + +from tests import unittest +from tests.utils import setup_test_homeserver + + +class AuthHandlers(object): + def __init__(self, hs): + self.auth_handler = AuthHandler(hs) + + +class TestResourceLimitsServerNotices(unittest.TestCase): + @defer.inlineCallbacks + def setUp(self): + self.hs = yield setup_test_homeserver(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() + + # relying on [1] is far from ideal, but the only case where + # ResourceLimitsServerNotices class needs to be isolated is this test, + # general code should never have a reason to do so ... + self._rlsn = self.server_notices_sender._server_notices[1] + if not isinstance(self._rlsn, ResourceLimitsServerNotices): + raise Exception("Failed to find reference to ResourceLimitsServerNotices") + + self._rlsn._store.user_last_seen_monthly_active = Mock( + return_value=defer.succeed(1000) + ) + self._send_notice = self._rlsn._server_notices_manager.send_notice + self._rlsn._server_notices_manager.send_notice = Mock() + self._send_notice = self._rlsn._server_notices_manager.send_notice + + self._rlsn._limit_usage_by_mau = True + self.user_id = "user_id" + + @defer.inlineCallbacks + 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 + + yield self._rlsn.maybe_send_server_notice_to_user("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") + + self._send_notice.assert_not_called() + + @defer.inlineCallbacks + 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() + + 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) + + @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") + + 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") + + # "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) + + @defer.inlineCallbacks + 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() + + 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): + + """Test when user is not part of the MAU cohort - this should not ever + happen - but ... + """ + + self._rlsn.auth.check_auth_blocking = Mock() + self._rlsn._store.user_last_seen_monthly_active = Mock( + return_value=defer.succeed(None) + ) + 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) |