From 01021c812f86466a5f35db8afd443b9cb17bd092 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Thu, 9 Aug 2018 22:16:00 +0100
Subject: [PATCH 01/19] wip at implementing MSC 7075

---
 .../resource_limits_server_notices.py         | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 synapse/server_notices/resource_limits_server_notices.py

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
new file mode 100644
index 000000000..33b1d80cd
--- /dev/null
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -0,0 +1,84 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+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
+
+logger = logging.getLogger(__name__)
+
+
+class ResourceLimitsServerNotices(object):
+    """
+    """
+    def __init__(self, hs):
+        """
+
+        Args:
+            hs (synapse.server.HomeServer):
+        """
+        self._server_notices_manager = hs.get_server_notices_manager()
+        self._store = hs.get_datastore()
+        self._api = hs.get_api()
+        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._notified = set()
+        self._resouce_limited = False
+        # 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
+
+        Args:
+            user_id (str): user to check
+
+        Returns:
+            Deferred
+        """
+        if self._limit_usage_by_mau is False and self._hs_disabled is False:
+            # not enabled
+            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:
+                # Need to start removing notices
+                pass
+        except AuthError as e:
+            # Need to start notifying of blocking
+            if not self._resouce_limited:
+                pass
+
+            # need to send a message.
+            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)

From 6c6aba76e1059f103ed48487e74ab4161347638b Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Fri, 10 Aug 2018 15:12:59 +0100
Subject: [PATCH 02/19] implementation of server notices to alert on hitting
 resource limits

---
 .../resource_limits_server_notices.py         |  79 ++++++-----
 .../server_notices/server_notices_sender.py   |  33 +++--
 .../test_resource_limits_server_notices.py    | 125 ++++++++++++++++++
 3 files changed, 191 insertions(+), 46 deletions(-)
 create mode 100644 tests/server_notices/test_resource_limits_server_notices.py

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 33b1d80cd..017828f81 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:
-                # Need to start removing notices
-                pass
-        except AuthError as e:
-            # Need to start notifying of blocking
-            if not self._resouce_limited:
-                pass
-
-            # need to send a message.
+        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._server_notices_manager.send_notice(
-                    user_id, content,
-                )
+                yield self.auth.check_auth_blocking()
+                self._resouce_limited = False
+                # Need to start removing notices
+                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)
 
-            except SynapseError as e:
-                logger.error("Error sending server notice about resource limits: %s", e)
+            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)
+
+    @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 5d23965f3..6121b2f26 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 000000000..a69253f1b
--- /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)

From c79c4c0a7d4887c24050df72417b1be9afe2992c Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Fri, 10 Aug 2018 15:25:47 +0100
Subject: [PATCH 03/19] server notices for resource limit blocking

---
 changelog.d/3680.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/3680.feature

diff --git a/changelog.d/3680.feature b/changelog.d/3680.feature
new file mode 100644
index 000000000..4edaaf76a
--- /dev/null
+++ b/changelog.d/3680.feature
@@ -0,0 +1 @@
+Server notices for resource limit blocking

From 63417c31e9d32992fbb422b630024946494ff26b Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Mon, 13 Aug 2018 22:36:52 +0100
Subject: [PATCH 04/19] fix typo

---
 synapse/server_notices/resource_limits_server_notices.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 017828f81..e4be83643 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -34,7 +34,7 @@ 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._limit_usage_by_mau = hs.config.limit_usage_by_mau = False
+        self._limit_usage_by_mau = hs.config.limit_usage_by_mau
         self._hs_disabled = hs.config.hs_disabled
 
         self._notified_of_blocking = set()

From 9b75c78b4df276fe40d3f1b92a57481c3e970886 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Tue, 14 Aug 2018 11:20:41 +0100
Subject: [PATCH 05/19] support server notice state events for resource limits

---
 synapse/api/constants.py                      |  2 ++
 .../server_notices/consent_server_notices.py  |  3 ++-
 .../resource_limits_server_notices.py         | 21 ++++++++++++++-----
 .../server_notices/server_notices_manager.py  |  4 ++--
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index b0da506f6..ef6084199 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -79,6 +79,8 @@ class EventTypes(object):
 
     ServerACL = "m.room.server_acl"
 
+    ServerNoticeLimitReached = "m.server_notice.usage_limit_reached"
+
 
 class RejectedReason(object):
     AUTH_ERROR = "auth_error"
diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py
index 5e3044d16..6d5caedb0 100644
--- a/synapse/server_notices/consent_server_notices.py
+++ b/synapse/server_notices/consent_server_notices.py
@@ -22,6 +22,7 @@ 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__)
 
@@ -103,7 +104,7 @@ class ConsentServerNotices(object):
                     },
                 )
                 yield self._server_notices_manager.send_notice(
-                    user_id, content,
+                    user_id, content, EventTypes.Message
                 )
                 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 e4be83643..2f49dae16 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -17,6 +17,7 @@ import logging
 from twisted.internet import defer
 
 from synapse.api.errors import AuthError, SynapseError
+from synapse.api.constants import EventTypes
 
 logger = logging.getLogger(__name__)
 
@@ -61,22 +62,32 @@ class ResourceLimitsServerNotices(object):
                 # 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:
                     # Send message to remove warning - needs updating
-                    content = "remove warning"
+                    content = {
+                        'body': '',
+                        'admin_email': '',
+                    }
                     self._send_server_notice(user_id, content)
                     self._notified_of_blocking.remove(user_id)
 
-            except AuthError:
+            except AuthError as e:
                 # 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"
+                    # TODO use admin email contained in error once PR lands
+                    content = {
+                        'body': e.msg,
+                        'admin_email': 'stunt@adminemail.com',
+                        'msgtype': 'm.text'
+                    }
                     self._send_server_notice(user_id, content)
                     self._notified_of_blocking.add(user_id)
 
@@ -93,7 +104,7 @@ class ResourceLimitsServerNotices(object):
         """
         try:
             yield self._server_notices_manager.send_notice(
-                user_id, content,
+                user_id, content, EventTypes.ServerNoticeLimitReached
             )
         except SynapseError as e:
             logger.error("Error sending server notice about resource limits: %s", e)
diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py
index a26deace5..4806977a8 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -46,7 +46,7 @@ class ServerNoticesManager(object):
         return self._config.server_notices_mxid is not None
 
     @defer.inlineCallbacks
-    def send_notice(self, user_id, event_content):
+    def send_notice(self, user_id, event_content, type):
         """Send a notice to the given user
 
         Creates the server notices room, if none exists.
@@ -67,7 +67,7 @@ class ServerNoticesManager(object):
 
         yield self._event_creation_handler.create_and_send_nonmember_event(
             requester, {
-                "type": EventTypes.Message,
+                "type": type,
                 "room_id": room_id,
                 "sender": system_mxid,
                 "content": event_content,

From e2c9fe0a6a6dbe4f81402b79b6605e17fb05e5d4 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Tue, 14 Aug 2018 13:32:56 +0100
Subject: [PATCH 06/19] backout ability to pass in event type to server notices

---
 synapse/server_notices/consent_server_notices.py         | 2 +-
 synapse/server_notices/resource_limits_server_notices.py | 3 ++-
 synapse/server_notices/server_notices_manager.py         | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py
index 6d5caedb0..783ceae1a 100644
--- a/synapse/server_notices/consent_server_notices.py
+++ b/synapse/server_notices/consent_server_notices.py
@@ -104,7 +104,7 @@ class ConsentServerNotices(object):
                     },
                 )
                 yield self._server_notices_manager.send_notice(
-                    user_id, content, EventTypes.Message
+                    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 2f49dae16..0470f6620 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -91,6 +91,7 @@ class ResourceLimitsServerNotices(object):
                     self._send_server_notice(user_id, content)
                     self._notified_of_blocking.add(user_id)
 
+
     @defer.inlineCallbacks
     def _send_server_notice(self, user_id, content):
         """Sends Server notice
@@ -104,7 +105,7 @@ class ResourceLimitsServerNotices(object):
         """
         try:
             yield self._server_notices_manager.send_notice(
-                user_id, content, EventTypes.ServerNoticeLimitReached
+                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_manager.py b/synapse/server_notices/server_notices_manager.py
index 4806977a8..a26deace5 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -46,7 +46,7 @@ class ServerNoticesManager(object):
         return self._config.server_notices_mxid is not None
 
     @defer.inlineCallbacks
-    def send_notice(self, user_id, event_content, type):
+    def send_notice(self, user_id, event_content):
         """Send a notice to the given user
 
         Creates the server notices room, if none exists.
@@ -67,7 +67,7 @@ class ServerNoticesManager(object):
 
         yield self._event_creation_handler.create_and_send_nonmember_event(
             requester, {
-                "type": type,
+                "type": EventTypes.Message,
                 "room_id": room_id,
                 "sender": system_mxid,
                 "content": event_content,

From c24fc9797bc0dd59c0d8131ea09c1178e39b76f6 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Wed, 15 Aug 2018 15:04:30 +0100
Subject: [PATCH 07/19] add new event types

---
 synapse/api/constants.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index ef6084199..1fb24578e 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -78,6 +78,7 @@ class EventTypes(object):
     Name = "m.room.name"
 
     ServerACL = "m.room.server_acl"
+    Pinned = "m.room.pinned_events"
 
     ServerNoticeLimitReached = "m.server_notice.usage_limit_reached"
 

From eabc5f827107a24717655e0eec376adf88b574c4 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Wed, 15 Aug 2018 15:04:52 +0100
Subject: [PATCH 08/19] wip cut at sending resource server notices

---
 .../resource_limits_server_notices.py         | 65 ++++++++++---------
 .../server_notices/server_notices_manager.py  | 30 ++++++---
 2 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 0470f6620..94d0f9818 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -40,6 +40,7 @@ class ResourceLimitsServerNotices(object):
 
         self._notified_of_blocking = set()
         self._resouce_limited = False
+
         # Config checks?
 
     @defer.inlineCallbacks
@@ -69,43 +70,43 @@ class ResourceLimitsServerNotices(object):
                 self._resouce_limited = False
                 # Need to start removing notices
                 if user_id in self._notified_of_blocking:
-                    # Send message to remove warning - needs updating
+                    # Send message to remove warning
+                    # send state event here
+                    # How do I do this? if drop the id, how to refer to it?
                     content = {
-                        'body': '',
-                        'admin_email': '',
+                        "pinned":[]
                     }
-                    self._send_server_notice(user_id, content)
+                    yield self._server_notices_manager.send_notice(
+                        user_id, content, EventTypes.Pinned, '',
+                    )
+
                     self._notified_of_blocking.remove(user_id)
 
             except AuthError as e:
                 # Need to start notifying of blocking
+                try:
+                    self._resouce_limited = True
+                    if user_id not in self._notified_of_blocking:
+                        # TODO use admin email contained in error once PR lands
+                        content = {
+                            'body': e.msg,
+                            'admin_email': 'stunt@adminemail.com',
+                        }
+                        event = yield self._server_notices_manager.send_notice(
+                            user_id, content, EventTypes.ServerNoticeLimitReached
+                        )
 
-                self._resouce_limited = True
-                if user_id not in self._notified_of_blocking:
-                    # TODO use admin email contained in error once PR lands
-                    content = {
-                        'body': e.msg,
-                        'admin_email': 'stunt@adminemail.com',
-                        'msgtype': 'm.text'
-                    }
-                    self._send_server_notice(user_id, content)
-                    self._notified_of_blocking.add(user_id)
+                        # send server notices state event here
+                        # TODO Over writing pinned events
+                        content = {
+                            "pinned":[
+                                event.event_id,
+                            ]
+                        }
+                        yield self._server_notices_manager.send_notice(
+                            user_id, content, EventTypes.Pinned, '',
+                        )
 
-
-    @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)
+                        self._notified_of_blocking.add(user_id)
+                except SynapseError as e:
+                    logger.error("Error sending server notice about resource limits: %s", e)
diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py
index a26deace5..3e57f8211 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -46,7 +46,10 @@ class ServerNoticesManager(object):
         return self._config.server_notices_mxid is not None
 
     @defer.inlineCallbacks
-    def send_notice(self, user_id, event_content):
+    def send_notice(
+        self, user_id, event_content,
+        type=EventTypes.Message, state_key=None
+    ):
         """Send a notice to the given user
 
         Creates the server notices room, if none exists.
@@ -54,9 +57,11 @@ class ServerNoticesManager(object):
         Args:
             user_id (str): mxid of user to send event to.
             event_content (dict): content of event to send
+            type(EventTypes): type of event
+            is_state_event(bool): Is the event a state event
 
         Returns:
-            Deferred[None]
+            Deferred[FrozenEvent]
         """
         room_id = yield self.get_notice_room_for_user(user_id)
 
@@ -65,15 +70,19 @@ class ServerNoticesManager(object):
 
         logger.info("Sending server notice to %s", user_id)
 
-        yield self._event_creation_handler.create_and_send_nonmember_event(
-            requester, {
-                "type": EventTypes.Message,
-                "room_id": room_id,
-                "sender": system_mxid,
-                "content": event_content,
-            },
-            ratelimit=False,
+        event_dict = {
+            "type": type,
+            "room_id": room_id,
+            "sender": system_mxid,
+            "content": event_content,
+        }
+        if state_key:
+            event_dict['state_key'] = state_key
+
+        res = yield self._event_creation_handler.create_and_send_nonmember_event(
+            requester, event_dict, ratelimit=False,
         )
+        defer.returnValue(res)
 
     @cachedInlineCallbacks()
     def get_notice_room_for_user(self, user_id):
@@ -141,6 +150,7 @@ class ServerNoticesManager(object):
             creator_join_profile=join_profile,
         )
         room_id = info['room_id']
+        yield self._store.add_tag_to_room(user_id, room_id, 'm.server_notice', None)
 
         logger.info("Created server notices room %s for %s", room_id, user_id)
         defer.returnValue(room_id)

From c055c91655cbe5d4e6b36d86a00dafb02c585b6d Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 11:10:19 +0100
Subject: [PATCH 09/19] fix case where empty string state check is evaulated as
 False

---
 synapse/server_notices/server_notices_manager.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py
index 3e57f8211..22c819cc5 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -76,7 +76,8 @@ class ServerNoticesManager(object):
             "sender": system_mxid,
             "content": event_content,
         }
-        if state_key:
+
+        if state_key is not None:
             event_dict['state_key'] = state_key
 
         res = yield self._event_creation_handler.create_and_send_nonmember_event(

From df1e4f259fda3009dacbb3211e999f7d89adaf87 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 11:10:53 +0100
Subject: [PATCH 10/19] WIP impl commiting to get feedback

---
 .../resource_limits_server_notices.py         | 80 +++++++++++++++++--
 1 file changed, 74 insertions(+), 6 deletions(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 94d0f9818..2b714a101 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -40,7 +40,8 @@ class ResourceLimitsServerNotices(object):
 
         self._notified_of_blocking = set()
         self._resouce_limited = False
-
+        self._message_handler = hs.get_message_handler()
+        self._state = hs.get_state_handler()
         # Config checks?
 
     @defer.inlineCallbacks
@@ -57,6 +58,72 @@ 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)
+
+
+            # 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'))
+
+            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')
+
+            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':
+                    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.
@@ -69,7 +136,8 @@ class ResourceLimitsServerNotices(object):
                 yield self.auth.check_auth_blocking()
                 self._resouce_limited = False
                 # Need to start removing notices
-                if user_id in self._notified_of_blocking:
+                # if user_id in self._notified_of_blocking:
+                if currently_blocked:
                     # Send message to remove warning
                     # send state event here
                     # How do I do this? if drop the id, how to refer to it?
@@ -79,14 +147,14 @@ class ResourceLimitsServerNotices(object):
                     yield self._server_notices_manager.send_notice(
                         user_id, content, EventTypes.Pinned, '',
                     )
-
-                    self._notified_of_blocking.remove(user_id)
+                    logger.info('deactivate block')
 
             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 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,
@@ -103,10 +171,10 @@ class ResourceLimitsServerNotices(object):
                                 event.event_id,
                             ]
                         }
+                        logger.info("active block")
                         yield self._server_notices_manager.send_notice(
                             user_id, content, EventTypes.Pinned, '',
                         )
 
-                        self._notified_of_blocking.add(user_id)
                 except SynapseError as e:
                     logger.error("Error sending server notice about resource limits: %s", e)

From a675f9c5560a16655a9337992499690c9f27a7eb Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 14:53:35 +0100
Subject: [PATCH 11/19] check for room state before deciding on action

---
 .../server_notices/consent_server_notices.py  |   1 -
 .../resource_limits_server_notices.py         | 110 ++++++------------
 .../test_resource_limits_server_notices.py    |  52 +++++----
 3 files changed, 67 insertions(+), 96 deletions(-)

diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py
index 783ceae1a..a9fe58d9f 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 2b714a101..43a926fc7 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)
-
-
-            # 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'))
-
-            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')
-
-            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':
-                    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
+
+            room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
+
+            currently_blocked = False
+            logger.info("GET CURRENT STATE")
+            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:
+                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 == EventTypes.ServerNoticeLimitReached:
+                    currently_blocked = True
+
+            logger.info("currently_blocked is %r" % currently_blocked)
             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 a69253f1b..fb8c593a7 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)

From eff3ae3b9a8ab558a106ce3fe69774a2e594cf72 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 15:48:34 +0100
Subject: [PATCH 12/19] add room tagging

---
 .../resource_limits_server_notices.py              | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 43a926fc7..570e8307c 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -66,8 +66,12 @@ class ResourceLimitsServerNotices(object):
 
             room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
 
+            # Need to set tag here because room may have been created prior to
+            # tagging being set on creation. Ideally would set something to read
+            # room tags first, and cache that aggressively/
+            yield self._store.add_tag_to_room(user_id, room_id, 'm.server_notice', None)
+
             currently_blocked = False
-            logger.info("GET CURRENT STATE")
             pinned_state_event = None
             try:
                 pinned_state_event = yield self._state.get_current_state(
@@ -82,15 +86,9 @@ class ResourceLimitsServerNotices(object):
                 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 == EventTypes.ServerNoticeLimitReached:
                     currently_blocked = True
-
-            logger.info("currently_blocked is %r" % currently_blocked)
             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
@@ -109,7 +107,6 @@ class ResourceLimitsServerNotices(object):
                     yield self._server_notices_manager.send_notice(
                         user_id, content, EventTypes.Pinned, '',
                     )
-                    logger.info('deactivate block')
 
             except AuthError as e:
                 # Need to start notifying of blocking
@@ -131,7 +128,6 @@ class ResourceLimitsServerNotices(object):
                                 event.event_id,
                             ]
                         }
-                        logger.info("active block")
                         yield self._server_notices_manager.send_notice(
                             user_id, content, EventTypes.Pinned, '',
                         )

From 3c1080b6e4a4d7c80d67bd9bee45d845aa145088 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 17:02:04 +0100
Subject: [PATCH 13/19] refactor for readability, and reuse caching for setting
 tags

---
 .../resource_limits_server_notices.py         | 166 +++++++++++-------
 .../server_notices/server_notices_manager.py  |   6 +-
 .../test_resource_limits_server_notices.py    |   1 +
 3 files changed, 110 insertions(+), 63 deletions(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 570e8307c..61becca75 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -18,6 +18,7 @@ from twisted.internet import defer
 
 from synapse.api.constants import EventTypes
 from synapse.api.errors import AuthError, SynapseError
+from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG
 
 logger = logging.getLogger(__name__)
 
@@ -57,80 +58,121 @@ class ResourceLimitsServerNotices(object):
         if self._hs_disabled is True:
             return
 
-        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
+        if self._limit_usage_by_mau is False:
+            return
 
-            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
 
-            # Need to set tag here because room may have been created prior to
-            # tagging being set on creation. Ideally would set something to read
-            # room tags first, and cache that aggressively/
-            yield self._store.add_tag_to_room(user_id, room_id, 'm.server_notice', None)
+        # Determine current state of room
 
-            currently_blocked = False
-            pinned_state_event = None
-            try:
-                pinned_state_event = yield self._state.get_current_state(
-                    room_id, event_type=EventTypes.Pinned
+        room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
+
+        yield self._check_and_set_tags(user_id, room_id)
+        currently_blocked, ref_events = yield self._is_room_currently_blocked(room_id)
+
+        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()
+            if currently_blocked:
+                # Room is notifying of a block, when it ought not to be.
+                # Remove block notification
+                content = {
+                    "pinned": ref_events
+                }
+                yield self._server_notices_manager.send_notice(
+                    user_id, content, 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:
-                referenced_events = pinned_state_event.content.get('pinned')
+        except AuthError as e:
 
-            events = yield self._store.get_events(referenced_events)
-            for event_id, event in events.items():
-                if event.type == EventTypes.ServerNoticeLimitReached:
-                    currently_blocked = True
             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()
-
-                # Need to start removing notices
-                # if user_id in self._notified_of_blocking:
-                if currently_blocked:
-                    # Send message to remove warning
-                    # send state event here
-                    # How do I do this? if drop the id, how to refer to it?
+                if not currently_blocked:
+                    # Room is not notifying of a block, when it ought to be.
+                    # Add block notification
                     content = {
-                        "pinned": []
+                        '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 AuthError as e:
-                # Need to start notifying of blocking
-                try:
-                    if not currently_blocked:
-                        # TODO use admin email contained in error once PR lands
-                        content = {
-                            'body': e.msg,
-                            'admin_uri': self._admin_uri,
-                        }
-                        event = yield self._server_notices_manager.send_notice(
-                            user_id, content, EventTypes.ServerNoticeLimitReached
-                        )
+            except SynapseError as e:
+                logger.error("Error sending resource limits server notice: %s", e)
 
-                        # send server notices state event here
-                        # TODO Over writing pinned events
-                        content = {
-                            "pinned": [
-                                event.event_id,
-                            ]
-                        }
-                        yield self._server_notices_manager.send_notice(
-                            user_id, content, EventTypes.Pinned, '',
-                        )
+    @defer.inlineCallbacks
+    def _check_and_set_tags(self, user_id, room_id):
+        """
+        Since server notices rooms were originally not with tags,
+        important to check that tags have been set correctly
+        Args:
+            user_id(str): the user in question
+            room_id(str): the server notices room for that user
+        """
+        tags = yield self._store.get_tags_for_user(user_id)
+        server_notices_tags = tags.get(room_id)
+        need_to_set_tag = True
+        if server_notices_tags:
+            if server_notice_tags.get(SERVER_NOTICE_ROOM_TAG):
+                # tag already present, nothing to do here
+                need_to_set_tag = False
+        if need_to_set_tag:
+            yield self._store.add_tag_to_room(
+                user_id, room_id, SERVER_NOTICE_ROOM_TAG, None
+            )
 
-                except SynapseError as e:
-                    logger.error("Error sending resource limits server notice: %s", e)
+    @defer.inlineCallbacks
+    def _is_room_currently_blocked(self, room_id):
+        """
+        Determines if the room is currently blocked
+
+        Args:
+            room_id(str): The room id of the server notices room
+
+        Returns:
+
+            bool: Is the room currently blocked
+            list: The list of pinned events that are unrelated to limit blocking
+            This list can be used as a convenience in the case where the block
+            is to be lifted and the remaining pinned event references need to be
+            preserved
+        """
+        currently_blocked = False
+        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:
+            referenced_events = pinned_state_event.content.get('pinned')
+
+        events = yield self._store.get_events(referenced_events)
+        event_to_remove = None
+        for event_id, event in events.items():
+            if event.type == EventTypes.ServerNoticeLimitReached:
+                currently_blocked = True
+                # remove event in case we need to disable blocking later on.
+                if event_id in referenced_events:
+                    referenced_events.remove(event.event_id)
+
+        defer.returnValue((currently_blocked, referenced_events))
diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py
index 22c819cc5..5968104a9 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -22,6 +22,8 @@ from synapse.util.caches.descriptors import cachedInlineCallbacks
 
 logger = logging.getLogger(__name__)
 
+SERVER_NOTICE_ROOM_TAG = "m.server_notice"
+
 
 class ServerNoticesManager(object):
     def __init__(self, hs):
@@ -151,7 +153,9 @@ class ServerNoticesManager(object):
             creator_join_profile=join_profile,
         )
         room_id = info['room_id']
-        yield self._store.add_tag_to_room(user_id, room_id, 'm.server_notice', None)
+        yield self._store.add_tag_to_room(
+            user_id, room_id, SERVER_NOTICE_ROOM_TAG, None
+        )
 
         logger.info("Created server notices room %s for %s", room_id, user_id)
         defer.returnValue(room_id)
diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py
index fb8c593a7..ccb69097b 100644
--- a/tests/server_notices/test_resource_limits_server_notices.py
+++ b/tests/server_notices/test_resource_limits_server_notices.py
@@ -54,6 +54,7 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         self._rlsn._server_notices_manager.get_notice_room_for_user = Mock(
             returnValue=""
         )
+        self._rlsn._store.add_tag_to_room = Mock()
         self.hs.config.admin_uri = "mailto:user@test.com"
 
     @defer.inlineCallbacks

From 51b17ec56615b710f4937e3c20208841a6591477 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Thu, 16 Aug 2018 17:32:22 +0100
Subject: [PATCH 14/19] flake8

---
 synapse/server_notices/resource_limits_server_notices.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 61becca75..33625fba7 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -128,7 +128,7 @@ class ResourceLimitsServerNotices(object):
         server_notices_tags = tags.get(room_id)
         need_to_set_tag = True
         if server_notices_tags:
-            if server_notice_tags.get(SERVER_NOTICE_ROOM_TAG):
+            if server_notices_tags.get(SERVER_NOTICE_ROOM_TAG):
                 # tag already present, nothing to do here
                 need_to_set_tag = False
         if need_to_set_tag:
@@ -158,7 +158,7 @@ class ResourceLimitsServerNotices(object):
             pinned_state_event = yield self._state.get_current_state(
                 room_id, event_type=EventTypes.Pinned
             )
-        except AuthError as e:
+        except AuthError:
             # The user has yet to join the server notices room
             pass
 
@@ -167,7 +167,6 @@ class ResourceLimitsServerNotices(object):
             referenced_events = pinned_state_event.content.get('pinned')
 
         events = yield self._store.get_events(referenced_events)
-        event_to_remove = None
         for event_id, event in events.items():
             if event.type == EventTypes.ServerNoticeLimitReached:
                 currently_blocked = True

From d49b77404beab498ad8c16bd55e811b423851240 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@matrix.org>
Date: Fri, 17 Aug 2018 15:21:34 +0100
Subject: [PATCH 15/19] clean up, no functional changes

---
 .../server_notices/consent_server_notices.py  |  2 +-
 .../resource_limits_server_notices.py         | 84 ++++++++++---------
 .../test_resource_limits_server_notices.py    | 32 +++----
 3 files changed, 63 insertions(+), 55 deletions(-)

diff --git a/synapse/server_notices/consent_server_notices.py b/synapse/server_notices/consent_server_notices.py
index a9fe58d9f..5e3044d16 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 33625fba7..5987e448b 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, '',
+                )
 
-                    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)
+        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 ccb69097b..cc8d2f539 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)
         )

From ba1fbf7d5bd8807e892de6de4bc82cb5e49581bd Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Sat, 18 Aug 2018 12:31:08 +0100
Subject: [PATCH 16/19] special case server_notices_mxid

---
 synapse/server_notices/resource_limits_server_notices.py | 2 +-
 tests/api/test_auth.py                                   | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 5987e448b..84b91aeb5 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -173,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.iteritems():
+        for event_id, event in iteritems(events):
             if event.type == EventTypes.ServerNoticeLimitReached:
                 currently_blocked = True
                 # remove event in case we need to disable blocking later on.
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index 022d81ce3..c4cbff4e8 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -476,3 +476,11 @@ class AuthTestCase(unittest.TestCase):
         self.assertEquals(e.exception.admin_uri, self.hs.config.admin_uri)
         self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
         self.assertEquals(e.exception.code, 403)
+
+    @defer.inlineCallbacks
+    def test_server_notices_mxid_special_cased(self):
+        self.hs.config.hs_disabled = True
+        user = "@user:server"
+        self.hs.config.server_notices_mxid = user
+        self.hs.config.hs_disabled_message = "Reason for being disabled"
+        yield self.auth.check_auth_blocking(user)

From c5171bf171ae28596c4e101e9dcc61bad7bcae63 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Sat, 18 Aug 2018 12:33:07 +0100
Subject: [PATCH 17/19] special case server_notices_mxid

---
 synapse/api/auth.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 022211e34..55384d6ff 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -783,6 +783,12 @@ class Auth(object):
             user_id(str|None): If present, checks for presence against existing
             MAU cohort
         """
+
+        # Never fail an auth check for the server notices users
+        # This can be a problem where event creation is prohibited due to blocking
+        if user_id == self.hs.config.server_notices_mxid:
+            return
+
         if self.hs.config.hs_disabled:
             raise ResourceLimitError(
                 403, self.hs.config.hs_disabled_message,

From e07970165f852ccbc4542f1aaf0fd1b2bc54b973 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Sat, 18 Aug 2018 14:39:45 +0100
Subject: [PATCH 18/19] rename error code

---
 synapse/api/auth.py         | 4 ++--
 synapse/api/errors.py       | 4 ++--
 tests/api/test_auth.py      | 4 ++--
 tests/handlers/test_sync.py | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 55384d6ff..4207a48af 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -792,7 +792,7 @@ class Auth(object):
         if self.hs.config.hs_disabled:
             raise ResourceLimitError(
                 403, self.hs.config.hs_disabled_message,
-                errcode=Codes.RESOURCE_LIMIT_EXCEED,
+                errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
                 admin_uri=self.hs.config.admin_uri,
                 limit_type=self.hs.config.hs_disabled_limit_type
             )
@@ -809,6 +809,6 @@ class Auth(object):
                     403, "Monthly Active User Limit Exceeded",
 
                     admin_uri=self.hs.config.admin_uri,
-                    errcode=Codes.RESOURCE_LIMIT_EXCEED,
+                    errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
                     limit_type="monthly_active_user"
                 )
diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index e26001ab1..c4ddba988 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -56,7 +56,7 @@ class Codes(object):
     SERVER_NOT_TRUSTED = "M_SERVER_NOT_TRUSTED"
     CONSENT_NOT_GIVEN = "M_CONSENT_NOT_GIVEN"
     CANNOT_LEAVE_SERVER_NOTICE_ROOM = "M_CANNOT_LEAVE_SERVER_NOTICE_ROOM"
-    RESOURCE_LIMIT_EXCEED = "M_RESOURCE_LIMIT_EXCEED"
+    RESOURCE_LIMIT_EXCEEDED = "M_RESOURCE_LIMIT_EXCEEDED"
     UNSUPPORTED_ROOM_VERSION = "M_UNSUPPORTED_ROOM_VERSION"
     INCOMPATIBLE_ROOM_VERSION = "M_INCOMPATIBLE_ROOM_VERSION"
 
@@ -238,7 +238,7 @@ class ResourceLimitError(SynapseError):
     """
     def __init__(
         self, code, msg,
-        errcode=Codes.RESOURCE_LIMIT_EXCEED,
+        errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
         admin_uri=None,
         limit_type=None,
     ):
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index c4cbff4e8..ed960090c 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -458,7 +458,7 @@ class AuthTestCase(unittest.TestCase):
         with self.assertRaises(ResourceLimitError) as e:
             yield self.auth.check_auth_blocking()
         self.assertEquals(e.exception.admin_uri, self.hs.config.admin_uri)
-        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
+        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
         self.assertEquals(e.exception.code, 403)
 
         # Ensure does not throw an error
@@ -474,7 +474,7 @@ class AuthTestCase(unittest.TestCase):
         with self.assertRaises(ResourceLimitError) as e:
             yield self.auth.check_auth_blocking()
         self.assertEquals(e.exception.admin_uri, self.hs.config.admin_uri)
-        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
+        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
         self.assertEquals(e.exception.code, 403)
 
     @defer.inlineCallbacks
diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py
index a01ab471f..31f54bbd7 100644
--- a/tests/handlers/test_sync.py
+++ b/tests/handlers/test_sync.py
@@ -51,7 +51,7 @@ class SyncTestCase(tests.unittest.TestCase):
         self.hs.config.hs_disabled = True
         with self.assertRaises(ResourceLimitError) as e:
             yield self.sync_handler.wait_for_sync_for_user(sync_config)
-        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
+        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
 
         self.hs.config.hs_disabled = False
 
@@ -59,7 +59,7 @@ class SyncTestCase(tests.unittest.TestCase):
 
         with self.assertRaises(ResourceLimitError) as e:
             yield self.sync_handler.wait_for_sync_for_user(sync_config)
-        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEED)
+        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
 
     def _generate_sync_config(self, user_id):
         return SyncConfig(

From 9643a6f7f208f52febc4be7edb1e38f4ff077b4d Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 22 Aug 2018 17:00:29 +0100
Subject: [PATCH 19/19] Update notice format

---
 synapse/api/constants.py                           |  6 ++++--
 .../resource_limits_server_notices.py              | 14 +++++++++++---
 .../test_resource_limits_server_notices.py         | 12 +++++++++---
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index 1fb24578e..a67862f4e 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -80,8 +80,6 @@ class EventTypes(object):
     ServerACL = "m.room.server_acl"
     Pinned = "m.room.pinned_events"
 
-    ServerNoticeLimitReached = "m.server_notice.usage_limit_reached"
-
 
 class RejectedReason(object):
     AUTH_ERROR = "auth_error"
@@ -106,3 +104,7 @@ DEFAULT_ROOM_VERSION = "1"
 # vdh-test-version is a placeholder to get room versioning support working and tested
 # until we have a working v2.
 KNOWN_ROOM_VERSIONS = {"1", "vdh-test-version"}
+
+
+ServerNoticeMsgType = "m.server_notice"
+ServerNoticeLimitReached = "m.server_notice.usage_limit_reached"
diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 84b91aeb5..575697e54 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -18,7 +18,11 @@ from six import iteritems
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventTypes
+from synapse.api.constants import (
+    EventTypes,
+    ServerNoticeLimitReached,
+    ServerNoticeMsgType,
+)
 from synapse.api.errors import AuthError, ResourceLimitError, SynapseError
 from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG
 
@@ -102,11 +106,13 @@ class ResourceLimitsServerNotices(object):
                 # Add block notification
                 content = {
                     'body': event_content,
+                    'msgtype': ServerNoticeMsgType,
+                    'server_notice_type': ServerNoticeLimitReached,
                     'admin_uri': self._config.admin_uri,
                     'limit_type': event_limit_type
                 }
                 event = yield self._server_notices_manager.send_notice(
-                    user_id, content, EventTypes.ServerNoticeLimitReached
+                    user_id, content, EventTypes.Message,
                 )
 
                 content = {
@@ -174,7 +180,9 @@ class ResourceLimitsServerNotices(object):
 
         events = yield self._store.get_events(referenced_events)
         for event_id, event in iteritems(events):
-            if event.type == EventTypes.ServerNoticeLimitReached:
+            if event.type != EventTypes.Message:
+                continue
+            if event.content.get("msgtype") == ServerNoticeMsgType:
                 currently_blocked = True
                 # remove event in case we need to disable blocking later on.
                 if event_id in referenced_events:
diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py
index cc8d2f539..ca9b31128 100644
--- a/tests/server_notices/test_resource_limits_server_notices.py
+++ b/tests/server_notices/test_resource_limits_server_notices.py
@@ -2,7 +2,7 @@ from mock import Mock
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventTypes
+from synapse.api.constants import EventTypes, ServerNoticeMsgType
 from synapse.api.errors import ResourceLimitError
 from synapse.handlers.auth import AuthHandler
 from synapse.server_notices.resource_limits_server_notices import (
@@ -78,7 +78,10 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
         """Test when user has blocked notice, but should have it removed"""
 
         self._rlsn._auth.check_auth_blocking = Mock()
-        mock_event = Mock(type=EventTypes.ServerNoticeLimitReached)
+        mock_event = Mock(
+            type=EventTypes.Message,
+            content={"msgtype": ServerNoticeMsgType},
+        )
         self._rlsn._store.get_events = Mock(return_value=defer.succeed(
             {"123": mock_event}
         ))
@@ -94,7 +97,10 @@ class TestResourceLimitsServerNotices(unittest.TestCase):
             side_effect=ResourceLimitError(403, 'foo')
         )
 
-        mock_event = Mock(type=EventTypes.ServerNoticeLimitReached)
+        mock_event = Mock(
+            type=EventTypes.Message,
+            content={"msgtype": ServerNoticeMsgType},
+        )
         self._rlsn._store.get_events = Mock(return_value=defer.succeed(
             {"123": mock_event}
         ))