Performance improvements and refactor of Ratelimiter (#7595)

While working on https://github.com/matrix-org/synapse/issues/5665 I found myself digging into the `Ratelimiter` class and seeing that it was both:

* Rather undocumented, and
* causing a *lot* of config checks

This PR attempts to refactor and comment the `Ratelimiter` class, as well as encourage config file accesses to only be done at instantiation. 

Best to be reviewed commit-by-commit.

5 files changed, 48 insertions, 37 deletions

diff --git a/tests/rest/client/v1/test_events.py b/tests/rest/client/v1/test_events.py
index b54b06482b..f75520877f 100644
--- a/tests/rest/client/v1/test_events.py
+++ b/tests/rest/client/v1/test_events.py
@@ -15,7 +15,7 @@
 
 """ Tests REST events for /events paths."""
 
-from mock import Mock, NonCallableMock
+from mock import Mock
 
 import synapse.rest.admin
 from synapse.rest.client.v1 import events, login, room
@@ -40,11 +40,7 @@ class EventStreamPermissionsTestCase(unittest.HomeserverTestCase):
         config["enable_registration"] = True
         config["auto_join_rooms"] = []
 
-        hs = self.setup_test_homeserver(
-            config=config, ratelimiter=NonCallableMock(spec_set=["can_do_action"])
-        )
-        self.ratelimiter = hs.get_ratelimiter()
-        self.ratelimiter.can_do_action.return_value = (True, 0)
+        hs = self.setup_test_homeserver(config=config)
 
         hs.get_handlers().federation_handler = Mock()
 
diff --git a/tests/rest/client/v1/test_login.py b/tests/rest/client/v1/test_login.py
index 0f0f7ca72d..9033f09fd2 100644
--- a/tests/rest/client/v1/test_login.py
+++ b/tests/rest/client/v1/test_login.py
@@ -29,7 +29,6 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
     ]
 
     def make_homeserver(self, reactor, clock):
-
         self.hs = self.setup_test_homeserver()
         self.hs.config.enable_registration = True
         self.hs.config.registrations_require_3pid = []
@@ -38,10 +37,20 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
 
         return self.hs
 
+    @override_config(
+        {
+            "rc_login": {
+                "address": {"per_second": 0.17, "burst_count": 5},
+                # Prevent the account login ratelimiter from raising first
+                #
+                # This is normally covered by the default test homeserver config
+                # which sets these values to 10000, but as we're overriding the entire
+                # rc_login dict here, we need to set this manually as well
+                "account": {"per_second": 10000, "burst_count": 10000},
+            }
+        }
+    )
     def test_POST_ratelimiting_per_address(self):
-        self.hs.config.rc_login_address.burst_count = 5
-        self.hs.config.rc_login_address.per_second = 0.17
-
         # Create different users so we're sure not to be bothered by the per-user
         # ratelimiter.
         for i in range(0, 6):
@@ -80,10 +89,20 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
 
         self.assertEquals(channel.result["code"], b"200", channel.result)
 
+    @override_config(
+        {
+            "rc_login": {
+                "account": {"per_second": 0.17, "burst_count": 5},
+                # Prevent the address login ratelimiter from raising first
+                #
+                # This is normally covered by the default test homeserver config
+                # which sets these values to 10000, but as we're overriding the entire
+                # rc_login dict here, we need to set this manually as well
+                "address": {"per_second": 10000, "burst_count": 10000},
+            }
+        }
+    )
     def test_POST_ratelimiting_per_account(self):
-        self.hs.config.rc_login_account.burst_count = 5
-        self.hs.config.rc_login_account.per_second = 0.17
-
         self.register_user("kermit", "monkey")
 
         for i in range(0, 6):
@@ -119,10 +138,20 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
 
         self.assertEquals(channel.result["code"], b"200", channel.result)
 
+    @override_config(
+        {
+            "rc_login": {
+                # Prevent the address login ratelimiter from raising first
+                #
+                # This is normally covered by the default test homeserver config
+                # which sets these values to 10000, but as we're overriding the entire
+                # rc_login dict here, we need to set this manually as well
+                "address": {"per_second": 10000, "burst_count": 10000},
+                "failed_attempts": {"per_second": 0.17, "burst_count": 5},
+            }
+        }
+    )
     def test_POST_ratelimiting_per_account_failed_attempts(self):
-        self.hs.config.rc_login_failed_attempts.burst_count = 5
-        self.hs.config.rc_login_failed_attempts.per_second = 0.17
-
         self.register_user("kermit", "monkey")
 
         for i in range(0, 6):
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 7dd86d0c27..4886bbb401 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -20,7 +20,7 @@
 
 import json
 
-from mock import Mock, NonCallableMock
+from mock import Mock
 from six.moves.urllib import parse as urlparse
 
 from twisted.internet import defer
@@ -46,13 +46,8 @@ class RoomBase(unittest.HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
 
         self.hs = self.setup_test_homeserver(
-            "red",
-            http_client=None,
-            federation_client=Mock(),
-            ratelimiter=NonCallableMock(spec_set=["can_do_action"]),
+            "red", http_client=None, federation_client=Mock(),
         )
-        self.ratelimiter = self.hs.get_ratelimiter()
-        self.ratelimiter.can_do_action.return_value = (True, 0)
 
         self.hs.get_federation_handler = Mock(return_value=Mock())
 
diff --git a/tests/rest/client/v1/test_typing.py b/tests/rest/client/v1/test_typing.py
index 4bc3aaf02d..18260bb90e 100644
--- a/tests/rest/client/v1/test_typing.py
+++ b/tests/rest/client/v1/test_typing.py
@@ -16,7 +16,7 @@
 
 """Tests REST events for /rooms paths."""
 
-from mock import Mock, NonCallableMock
+from mock import Mock
 
 from twisted.internet import defer
 
@@ -39,17 +39,11 @@ class RoomTypingTestCase(unittest.HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
 
         hs = self.setup_test_homeserver(
-            "red",
-            http_client=None,
-            federation_client=Mock(),
-            ratelimiter=NonCallableMock(spec_set=["can_do_action"]),
+            "red", http_client=None, federation_client=Mock(),
         )
 
         self.event_source = hs.get_event_sources().sources["typing"]
 
-        self.ratelimiter = hs.get_ratelimiter()
-        self.ratelimiter.can_do_action.return_value = (True, 0)
-
         hs.get_handlers().federation_handler = Mock()
 
         def get_user_by_access_token(token=None, allow_guest=False):
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index 5637ce2090..7deaf5b24a 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -29,6 +29,7 @@ from synapse.rest.client.v1 import login, logout
 from synapse.rest.client.v2_alpha import account, account_validity, register, sync
 
 from tests import unittest
+from tests.unittest import override_config
 
 
 class RegisterRestServletTestCase(unittest.HomeserverTestCase):
@@ -146,10 +147,8 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
         self.assertEquals(channel.result["code"], b"403", channel.result)
         self.assertEquals(channel.json_body["error"], "Guest access is disabled")
 
+    @override_config({"rc_registration": {"per_second": 0.17, "burst_count": 5}})
     def test_POST_ratelimiting_guest(self):
-        self.hs.config.rc_registration.burst_count = 5
-        self.hs.config.rc_registration.per_second = 0.17
-
         for i in range(0, 6):
             url = self.url + b"?kind=guest"
             request, channel = self.make_request(b"POST", url, b"{}")
@@ -168,10 +167,8 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
 
         self.assertEquals(channel.result["code"], b"200", channel.result)
 
+    @override_config({"rc_registration": {"per_second": 0.17, "burst_count": 5}})
     def test_POST_ratelimiting(self):
-        self.hs.config.rc_registration.burst_count = 5
-        self.hs.config.rc_registration.per_second = 0.17
-
         for i in range(0, 6):
             params = {
                 "username": "kermit" + str(i),