diff options
-rw-r--r-- | changelog.d/3747.bugfix | 1 | ||||
-rw-r--r-- | synapse/server_notices/resource_limits_server_notices.py | 6 | ||||
-rw-r--r-- | tests/server_notices/test_resource_limits_server_notices.py | 66 |
3 files changed, 72 insertions, 1 deletions
diff --git a/changelog.d/3747.bugfix b/changelog.d/3747.bugfix new file mode 100644 index 0000000000..c41e2a1213 --- /dev/null +++ b/changelog.d/3747.bugfix @@ -0,0 +1 @@ +Fix bug where we resent "limit exceeded" server notices repeatedly diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py index 575697e54b..96eb97771f 100644 --- a/synapse/server_notices/resource_limits_server_notices.py +++ b/synapse/server_notices/resource_limits_server_notices.py @@ -76,6 +76,10 @@ class ResourceLimitsServerNotices(object): room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id) + if not room_id: + logger.warn("Failed to get server notices room") + return + yield self._check_and_set_tags(user_id, room_id) currently_blocked, ref_events = yield self._is_room_currently_blocked(room_id) @@ -176,7 +180,7 @@ class ResourceLimitsServerNotices(object): referenced_events = [] if pinned_state_event is not None: - referenced_events = pinned_state_event.content.get('pinned') + referenced_events = list(pinned_state_event.content.get('pinned', [])) events = yield self._store.get_events(referenced_events) for event_id, event in iteritems(events): diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index ca9b31128a..fede3f52a9 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -144,3 +144,69 @@ class TestResourceLimitsServerNotices(unittest.TestCase): yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) self._send_notice.assert_not_called() + + +class TestResourceLimitsServerNoticesWithRealRooms(unittest.TestCase): + @defer.inlineCallbacks + def setUp(self): + self.hs = yield setup_test_homeserver(self.addCleanup) + self.store = self.hs.get_datastore() + self.server_notices_sender = self.hs.get_server_notices_sender() + self.server_notices_manager = self.hs.get_server_notices_manager() + self.event_source = self.hs.get_event_sources() + + # 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.hs.config.limit_usage_by_mau = True + self.hs.config.hs_disabled = False + self.hs.config.max_mau_value = 5 + self.hs.config.server_notices_mxid = "@server:test" + self.hs.config.server_notices_mxid_display_name = None + self.hs.config.server_notices_mxid_avatar_url = None + self.hs.config.server_notices_room_name = "Test Server Notice Room" + + self.user_id = "@user_id:test" + + self.hs.config.admin_uri = "mailto:user@test.com" + + @defer.inlineCallbacks + def test_server_notice_only_sent_once(self): + self.store.get_monthly_active_count = Mock( + return_value=1000, + ) + + self.store.user_last_seen_monthly_active = Mock( + return_value=1000, + ) + + # Call the function multiple times to ensure we only send the notice once + yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) + yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) + yield self._rlsn.maybe_send_server_notice_to_user(self.user_id) + + # Now lets get the last load of messages in the service notice room and + # check that there is only one server notice + room_id = yield self.server_notices_manager.get_notice_room_for_user( + self.user_id, + ) + + token = yield self.event_source.get_current_token() + events, _ = yield self.store.get_recent_events_for_room( + room_id, limit=100, end_token=token.room_key, + ) + + count = 0 + for event in events: + if event.type != EventTypes.Message: + continue + if event.content.get("msgtype") != ServerNoticeMsgType: + continue + + count += 1 + + self.assertEqual(count, 1) |