Change @cache_in_self to use underscore-prefixed attributes

changelog.d/8565.misc

@@ -0,0 +1 @@
+Apply some internal changes to the way the `HomeServer` caches its internal attributes, to make it more idiomatic.

synapse/handlers/identity.py

@@ -354,7 +354,8 @@ async def send_threepid_validation(
             raise SynapseError(500, "An error was encountered when sending the email")
 
         token_expires = (
-            self.hs.clock.time_msec() + self.hs.config.email_validation_token_lifetime
+            self.hs.get_clock().time_msec()
+            + self.hs.config.email_validation_token_lifetime
         )
 
         await self.store.start_or_continue_validation_session(

synapse/rest/client/v2_alpha/account.py

@@ -114,7 +114,7 @@ async def on_POST(self, request):
                 # comments for request_token_inhibit_3pid_errors.
                 # Also wait for some random amount of time between 100ms and 1s to make it
                 # look like we did something.
-                await self.hs.clock.sleep(random.randint(1, 10) / 10)
+                await self.hs.get_clock().sleep(random.randint(1, 10) / 10)
                 return 200, {"sid": random_string(16)}
 
             raise SynapseError(400, "Email not found", Codes.THREEPID_NOT_FOUND)
@@ -385,7 +385,7 @@ async def on_POST(self, request):
                 # comments for request_token_inhibit_3pid_errors.
                 # Also wait for some random amount of time between 100ms and 1s to make it
                 # look like we did something.
-                await self.hs.clock.sleep(random.randint(1, 10) / 10)
+                await self.hs.get_clock().sleep(random.randint(1, 10) / 10)
                 return 200, {"sid": random_string(16)}
 
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
@@ -460,7 +460,7 @@ async def on_POST(self, request):
                 # comments for request_token_inhibit_3pid_errors.
                 # Also wait for some random amount of time between 100ms and 1s to make it
                 # look like we did something.
-                await self.hs.clock.sleep(random.randint(1, 10) / 10)
+                await self.hs.get_clock().sleep(random.randint(1, 10) / 10)
                 return 200, {"sid": random_string(16)}
 
             raise SynapseError(400, "MSISDN is already in use", Codes.THREEPID_IN_USE)

synapse/rest/client/v2_alpha/register.py

@@ -134,7 +134,7 @@ async def on_POST(self, request):
                 # comments for request_token_inhibit_3pid_errors.
                 # Also wait for some random amount of time between 100ms and 1s to make it
                 # look like we did something.
-                await self.hs.clock.sleep(random.randint(1, 10) / 10)
+                await self.hs.get_clock().sleep(random.randint(1, 10) / 10)
                 return 200, {"sid": random_string(16)}
 
             raise SynapseError(400, "Email is already in use", Codes.THREEPID_IN_USE)
@@ -209,7 +209,7 @@ async def on_POST(self, request):
                 # comments for request_token_inhibit_3pid_errors.
                 # Also wait for some random amount of time between 100ms and 1s to make it
                 # look like we did something.
-                await self.hs.clock.sleep(random.randint(1, 10) / 10)
+                await self.hs.get_clock().sleep(random.randint(1, 10) / 10)
                 return 200, {"sid": random_string(16)}
 
             raise SynapseError(

synapse/rest/key/v2/local_key_resource.py

@@ -66,7 +66,7 @@ class LocalKey(Resource):
 
     def __init__(self, hs):
         self.config = hs.config
-        self.clock = hs.clock
+        self.clock = hs.get_clock()
         self.update_response_body(self.clock.time_msec())
         Resource.__init__(self)
 

synapse/server.py

@@ -145,7 +145,8 @@ def cache_in_self(builder: T) -> T:
             "@cache_in_self can only be used on functions starting with `get_`"
         )
 
-    depname = builder.__name__[len("get_") :]
+    # get_attr -> _attr
+    depname = builder.__name__[len("get") :]
 
     building = [False]
 
@@ -233,11 +234,11 @@ def __init__(
         self._instance_id = random_string(5)
         self._instance_name = config.worker_name or "master"
 
-        self.clock = Clock(reactor)
+        self._clock = Clock(reactor)
         self.distributor = Distributor()
 
         self.registration_ratelimiter = Ratelimiter(
-            clock=self.clock,
+            clock=self._clock,
             rate_hz=config.rc_registration.per_second,
             burst_count=config.rc_registration.burst_count,
         )
@@ -299,8 +300,11 @@ def is_mine(self, domain_specific_string: DomainSpecificString) -> bool:
     def is_mine_id(self, string: str) -> bool:
         return string.split(":", 1)[1] == self.hostname
 
+    # The caching attribute for this is technically already set in __init__, but it's replicated for the sake of
+    # consistency
+    @cache_in_self
     def get_clock(self) -> Clock:
-        return self.clock
+        return Clock(self._reactor)
 
     def get_datastore(self) -> DataStore:
         if not self.datastores:
@@ -681,7 +685,7 @@ def get_replication_streams(self) -> Dict[str, Stream]:
 
     @cache_in_self
     def get_federation_ratelimiter(self) -> FederationRateLimiter:
-        return FederationRateLimiter(self.clock, config=self.config.rc_federation)
+        return FederationRateLimiter(self.get_clock(), config=self.config.rc_federation)
 
     @cache_in_self
     def get_module_api(self) -> ModuleApi:

tests/handlers/test_auth.py

@@ -52,7 +52,7 @@ def test_token_is_a_macaroon(self):
             self.fail("some_user was not in %s" % macaroon.inspect())
 
     def test_macaroon_caveats(self):
-        self.hs.clock.now = 5000
+        self.hs.get_clock().now = 5000
 
         token = self.macaroon_generator.generate_access_token("a_user")
         macaroon = pymacaroons.Macaroon.deserialize(token)
@@ -78,7 +78,7 @@ def verify_nonce(caveat):
 
     @defer.inlineCallbacks
     def test_short_term_login_token_gives_user_id(self):
-        self.hs.clock.now = 1000
+        self.hs.get_clock().now = 1000
 
         token = self.macaroon_generator.generate_short_term_login_token("a_user", 5000)
         user_id = yield defer.ensureDeferred(
@@ -87,7 +87,7 @@ def test_short_term_login_token_gives_user_id(self):
         self.assertEqual("a_user", user_id)
 
         # when we advance the clock, the token should be rejected
-        self.hs.clock.now = 6000
+        self.hs.get_clock().now = 6000
         with self.assertRaises(synapse.api.errors.AuthError):
             yield defer.ensureDeferred(
                 self.auth_handler.validate_short_term_login_token_and_get_user_id(token)

tests/replication/_base.py

@@ -69,7 +69,7 @@ def prepare(self, reactor, clock, hs):
         self.worker_hs.get_datastore().db_pool = hs.get_datastore().db_pool
 
         self.test_handler = self._build_replication_data_handler()
-        self.worker_hs.replication_data_handler = self.test_handler
+        self.worker_hs._replication_data_handler = self.test_handler
 
         repl_handler = ReplicationCommandHandler(self.worker_hs)
         self.client = ClientReplicationStreamProtocol(

tests/rest/client/v1/test_presence.py

@@ -33,13 +33,13 @@ class PresenceTestCase(unittest.HomeserverTestCase):
 
     def make_homeserver(self, reactor, clock):
 
+        ph = Mock()
+        ph.set_state.return_value = defer.succeed(None)
+
         hs = self.setup_test_homeserver(
-            "red", http_client=None, federation_client=Mock()
+            "red", http_client=None, federation_client=Mock(), presence_handler=ph
         )
 
-        hs.presence_handler = Mock()
-        hs.presence_handler.set_state.return_value = defer.succeed(None)
-
         return hs
 
     def test_put_presence(self):
@@ -56,7 +56,7 @@ def test_put_presence(self):
         self.render(request)
 
         self.assertEqual(channel.code, 200)
-        self.assertEqual(self.hs.presence_handler.set_state.call_count, 1)
+        self.assertEqual(self.hs.get_presence_handler().set_state.call_count, 1)
 
     def test_put_presence_disabled(self):
         """
@@ -72,4 +72,4 @@ def test_put_presence_disabled(self):
         self.render(request)
 
         self.assertEqual(channel.code, 200)
-        self.assertEqual(self.hs.presence_handler.set_state.call_count, 0)
+        self.assertEqual(self.hs.get_presence_handler().set_state.call_count, 0)

tests/rest/client/v2_alpha/test_register.py

@@ -598,7 +598,7 @@ def create_user(self):
         tok = self.login("kermit", "monkey")
         # We need to manually add an email address otherwise the handler will do
         # nothing.
-        now = self.hs.clock.time_msec()
+        now = self.hs.get_clock().time_msec()
         self.get_success(
             self.store.user_add_threepid(
                 user_id=user_id,
@@ -616,7 +616,7 @@ def test_manual_email_send_expired_account(self):
 
         # We need to manually add an email address otherwise the handler will do
         # nothing.
-        now = self.hs.clock.time_msec()
+        now = self.hs.get_clock().time_msec()
         self.get_success(
             self.store.user_add_threepid(
                 user_id=user_id,
@@ -676,7 +676,7 @@ def test_background_job(self):
 
         self.hs.config.account_validity.startup_job_max_delta = self.max_delta
 
-        now_ms = self.hs.clock.time_msec()
+        now_ms = self.hs.get_clock().time_msec()
         self.get_success(self.store._set_expiration_date_when_missing())
 
         res = self.get_success(self.store.get_expiration_ts_for_user(user_id))

tests/utils.py

@@ -271,7 +271,7 @@ def setup_test_homeserver(
 
     # Install @cache_in_self attributes
     for key, val in kwargs.items():
-        setattr(hs, key, val)
+        setattr(hs, "_" + key, val)
 
     # Mock TLS
     hs.tls_server_context_factory = Mock()