Solidify the HomeServer constructor. (#8515)

This implements a more standard API for instantiating a homeserver and
moves some of the dependency injection into the test suite.

More concretely this stops using `setattr` on all `kwargs` passed to `HomeServer`.

changelog.d/8515.misc

@@ -0,0 +1 @@
+Apply some internal fixes to the `HomeServer` class to make its code more idiomatic and statically-verifiable.

synapse/server.py

@@ -205,7 +205,13 @@ class HomeServer(metaclass=abc.ABCMeta):
     # instantiated during setup() for future return by get_datastore()
     DATASTORE_CLASS = abc.abstractproperty()
 
-    def __init__(self, hostname: str, config: HomeServerConfig, reactor=None, **kwargs):
+    def __init__(
+        self,
+        hostname: str,
+        config: HomeServerConfig,
+        reactor=None,
+        version_string="Synapse",
+    ):
         """
         Args:
             hostname : The hostname for the server.
@@ -236,11 +242,9 @@ class HomeServer(metaclass=abc.ABCMeta):
             burst_count=config.rc_registration.burst_count,
         )
 
-        self.datastores = None  # type: Optional[Databases]
+        self.version_string = version_string
 
-        # Other kwargs are explicit dependencies
+        self.datastores = None  # type: Optional[Databases]
-        for depname in kwargs:
-            setattr(self, depname, kwargs[depname])
 
     def get_instance_id(self) -> str:
         """A unique ID for this synapse process instance.

tests/app/test_frontend_proxy.py

@@ -22,7 +22,7 @@ class FrontendProxyTests(HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
 
         hs = self.setup_test_homeserver(
-            http_client=None, homeserverToUse=GenericWorkerServer
+            http_client=None, homeserver_to_use=GenericWorkerServer
         )
 
         return hs

tests/app/test_openid_listener.py

@@ -26,7 +26,7 @@ from tests.unittest import HomeserverTestCase
 class FederationReaderOpenIDListenerTests(HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
         hs = self.setup_test_homeserver(
-            http_client=None, homeserverToUse=GenericWorkerServer
+            http_client=None, homeserver_to_use=GenericWorkerServer
         )
         return hs
 
@@ -84,7 +84,7 @@ class FederationReaderOpenIDListenerTests(HomeserverTestCase):
 class SynapseHomeserverOpenIDListenerTests(HomeserverTestCase):
     def make_homeserver(self, reactor, clock):
         hs = self.setup_test_homeserver(
-            http_client=None, homeserverToUse=SynapseHomeServer
+            http_client=None, homeserver_to_use=SynapseHomeServer
         )
         return hs
 

tests/replication/_base.py

@@ -59,7 +59,7 @@ class BaseStreamTestCase(unittest.HomeserverTestCase):
         self.reactor.lookups["testserv"] = "1.2.3.4"
         self.worker_hs = self.setup_test_homeserver(
             http_client=None,
-            homeserverToUse=GenericWorkerServer,
+            homeserver_to_use=GenericWorkerServer,
             config=self._get_worker_hs_config(),
             reactor=self.reactor,
         )
@@ -266,7 +266,7 @@ class BaseMultiWorkerStreamTestCase(unittest.HomeserverTestCase):
         config.update(extra_config)
 
         worker_hs = self.setup_test_homeserver(
-            homeserverToUse=GenericWorkerServer,
+            homeserver_to_use=GenericWorkerServer,
             config=config,
             reactor=self.reactor,
             **kwargs

tests/replication/test_federation_ack.py

@@ -31,7 +31,7 @@ class FederationAckTestCase(HomeserverTestCase):
         return config
 
     def make_homeserver(self, reactor, clock):
-        hs = self.setup_test_homeserver(homeserverToUse=GenericWorkerServer)
+        hs = self.setup_test_homeserver(homeserver_to_use=GenericWorkerServer)
 
         return hs
 

tests/utils.py

@@ -21,6 +21,7 @@ import time
 import uuid
 import warnings
 from inspect import getcallargs
+from typing import Type
 from urllib import parse as urlparse
 
 from mock import Mock, patch
@@ -194,8 +195,8 @@ def setup_test_homeserver(
     name="test",
     config=None,
     reactor=None,
-    homeserverToUse=TestHomeServer,
+    homeserver_to_use: Type[HomeServer] = TestHomeServer,
-    **kargs
+    **kwargs
 ):
     """
     Setup a homeserver suitable for running tests against.  Keyword arguments
@@ -218,8 +219,8 @@ def setup_test_homeserver(
 
     config.ldap_enabled = False
 
-    if "clock" not in kargs:
+    if "clock" not in kwargs:
-        kargs["clock"] = MockClock()
+        kwargs["clock"] = MockClock()
 
     if USE_POSTGRES_FOR_TESTS:
         test_db = "synapse_test_%s" % uuid.uuid4().hex
@@ -264,18 +265,20 @@ def setup_test_homeserver(
         cur.close()
         db_conn.close()
 
-    hs = homeserverToUse(
+    hs = homeserver_to_use(
-        name,
+        name, config=config, version_string="Synapse/tests", reactor=reactor,
-        config=config,
-        version_string="Synapse/tests",
-        tls_server_context_factory=Mock(),
-        tls_client_options_factory=Mock(),
-        reactor=reactor,
-        **kargs
     )
 
+    # Install @cache_in_self attributes
+    for key, val in kwargs.items():
+        setattr(hs, key, val)
+
+    # Mock TLS
+    hs.tls_server_context_factory = Mock()
+    hs.tls_client_options_factory = Mock()
+
     hs.setup()
-    if homeserverToUse.__name__ == "TestHomeServer":
+    if homeserver_to_use == TestHomeServer:
         hs.setup_background_tasks()
 
     if isinstance(db_engine, PostgresEngine):
@@ -339,7 +342,7 @@ def setup_test_homeserver(
 
     hs.get_auth_handler().validate_hash = validate_hash
 
-    fed = kargs.get("resource_for_federation", None)
+    fed = kwargs.get("resource_for_federation", None)
     if fed:
         register_federation_servlets(hs, fed)