Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Change @cache_in_self to use underscore-prefixed attributes #8565

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8565.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Simplify the way the `HomeServer` object caches its internal attributes.
3 changes: 2 additions & 1 deletion synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
ShadowJonathan marked this conversation as resolved.
Show resolved Hide resolved
)

await self.store.start_or_continue_validation_session(
Expand Down
6 changes: 3 additions & 3 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/key/v2/local_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
27 changes: 13 additions & 14 deletions synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -233,15 +234,6 @@ def __init__(
self._instance_id = random_string(5)
self._instance_name = config.worker_name or "master"

self.clock = Clock(reactor)
self.distributor = Distributor()

self.registration_ratelimiter = Ratelimiter(
clock=self.clock,
rate_hz=config.rc_registration.per_second,
burst_count=config.rc_registration.burst_count,
)

self.version_string = version_string

self.datastores = None # type: Optional[Databases]
Expand Down Expand Up @@ -299,8 +291,9 @@ def is_mine(self, domain_specific_string: DomainSpecificString) -> bool:
def is_mine_id(self, string: str) -> bool:
return string.split(":", 1)[1] == self.hostname

@cache_in_self
def get_clock(self) -> Clock:
return self.clock
return Clock(self._reactor)

def get_datastore(self) -> DataStore:
if not self.datastores:
Expand All @@ -317,11 +310,17 @@ def get_datastores(self) -> Databases:
def get_config(self) -> HomeServerConfig:
return self.config

@cache_in_self
def get_distributor(self) -> Distributor:
return self.distributor
return Distributor()

@cache_in_self
def get_registration_ratelimiter(self) -> Ratelimiter:
return self.registration_ratelimiter
return Ratelimiter(
clock=self.get_clock(),
rate_hz=self.config.rc_registration.per_second,
burst_count=self.config.rc_registration.burst_count,
)

@cache_in_self
def get_federation_client(self) -> FederationClient:
Expand Down Expand Up @@ -681,7 +680,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:
Expand Down
6 changes: 3 additions & 3 deletions tests/handlers/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/replication/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can replication_data_handler be passed into setup_test_homeserver (similar to the changes made bleow in tests/rest/client/v1/test_presence.py)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot be easily done because _build_replication_data_handler instantiates TestReplicationDataHandler with the homeserver, so you end up with a chicken/egg problem.


repl_handler = ReplicationCommandHandler(self.worker_hs)
self.client = ClientReplicationStreamProtocol(
Expand Down
15 changes: 9 additions & 6 deletions tests/rest/client/v1/test_presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ class PresenceTestCase(unittest.HomeserverTestCase):

def make_homeserver(self, reactor, clock):

presence_handler = Mock()
presence_handler.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=presence_handler,
)

hs.presence_handler = Mock()
hs.presence_handler.set_state.return_value = defer.succeed(None)

return hs

def test_put_presence(self):
Expand All @@ -56,7 +59,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):
"""
Expand All @@ -72,4 +75,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)
6 changes: 3 additions & 3 deletions tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down