From 84a55a82ab5c0e176c2bef8eb966f2ff6f4024b2 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 09:27:50 +0000
Subject: [PATCH 01/10] Correct reactor type hints in HomeserverTestCase and
 children

---
 tests/handlers/test_typing.py        | 7 ++++++-
 tests/storage/test_user_directory.py | 5 ++++-
 tests/unittest.py                    | 3 ++-
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index efbb5a8dbbaf..b87ba2f7e035 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -29,6 +29,7 @@
 from synapse.util import Clock
 
 from tests import unittest
+from tests.server import ThreadedMemoryReactorClock
 from tests.test_utils import make_awaitable
 from tests.unittest import override_config
 
@@ -62,7 +63,11 @@ def _make_edu_transaction_json(edu_type: str, content: JsonDict) -> bytes:
 
 
 class TypingNotificationsTestCase(unittest.HomeserverTestCase):
-    def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
+    def make_homeserver(
+        self,
+        reactor: ThreadedMemoryReactorClock,
+        clock: Clock,
+    ) -> HomeServer:
         # we mock out the keyring so as to skip the authentication check on the
         # federation API call.
         mock_keyring = Mock(spec=["verify_json_for_server"])
diff --git a/tests/storage/test_user_directory.py b/tests/storage/test_user_directory.py
index 3ba896ecf384..f1ca523d23f9 100644
--- a/tests/storage/test_user_directory.py
+++ b/tests/storage/test_user_directory.py
@@ -28,6 +28,7 @@
 from synapse.storage.roommember import ProfileInfo
 from synapse.util import Clock
 
+from tests.server import ThreadedMemoryReactorClock
 from tests.test_utils.event_injection import inject_member_event
 from tests.unittest import HomeserverTestCase, override_config
 
@@ -138,7 +139,9 @@ class UserDirectoryInitialPopulationTestcase(HomeserverTestCase):
         register.register_servlets,
     ]
 
-    def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer:
+    def make_homeserver(
+        self, reactor: ThreadedMemoryReactorClock, clock: Clock
+    ) -> HomeServer:
         self.appservice = ApplicationService(
             token="i_am_an_app_service",
             id="1234",
diff --git a/tests/unittest.py b/tests/unittest.py
index a120c2976ccd..fa92dd94eb6a 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -75,6 +75,7 @@
 from tests.server import (
     CustomHeaderType,
     FakeChannel,
+    ThreadedMemoryReactorClock,
     get_clock,
     make_request,
     setup_test_homeserver,
@@ -360,7 +361,7 @@ def wait_for_background_updates(self) -> None:
                 store.db_pool.updates.do_next_background_update(False), by=0.1
             )
 
-    def make_homeserver(self, reactor: MemoryReactor, clock: Clock):
+    def make_homeserver(self, reactor: ThreadedMemoryReactorClock, clock: Clock):
         """
         Make and return a homeserver.
 

From 1fe5373a1469bec768211d14c77ade3bb8bc7e20 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 10:16:08 +0000
Subject: [PATCH 02/10] Fix type of limit arg to
 TypingEventSource.get_new_events

This must be an int, not a NoneType. What we provide here doesn't really
matter at the moment, as the argument isn't even used.
---
 tests/handlers/test_typing.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index b87ba2f7e035..53deecacdf33 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -191,7 +191,7 @@ def test_started_typing_local(self) -> None:
         self.assertEqual(self.event_source.get_current_key(), 1)
         events = self.get_success(
             self.event_source.get_new_events(
-                user=U_APPLE, from_key=0, limit=None, room_ids=[ROOM_ID], is_guest=False
+                user=U_APPLE, from_key=0, limit=0, room_ids=[ROOM_ID], is_guest=False
             )
         )
         self.assertEqual(
@@ -262,7 +262,7 @@ def test_started_typing_remote_recv(self) -> None:
         self.assertEqual(self.event_source.get_current_key(), 1)
         events = self.get_success(
             self.event_source.get_new_events(
-                user=U_APPLE, from_key=0, limit=None, room_ids=[ROOM_ID], is_guest=False
+                user=U_APPLE, from_key=0, limit=0, room_ids=[ROOM_ID], is_guest=False
             )
         )
         self.assertEqual(
@@ -303,7 +303,7 @@ def test_started_typing_remote_recv_not_in_room(self) -> None:
             self.event_source.get_new_events(
                 user=U_APPLE,
                 from_key=0,
-                limit=None,
+                limit=0,
                 room_ids=[OTHER_ROOM_ID],
                 is_guest=False,
             )
@@ -356,7 +356,7 @@ def test_stopped_typing(self) -> None:
         self.assertEqual(self.event_source.get_current_key(), 1)
         events = self.get_success(
             self.event_source.get_new_events(
-                user=U_APPLE, from_key=0, limit=None, room_ids=[ROOM_ID], is_guest=False
+                user=U_APPLE, from_key=0, limit=0, room_ids=[ROOM_ID], is_guest=False
             )
         )
         self.assertEqual(
@@ -392,7 +392,7 @@ def test_typing_timeout(self) -> None:
             self.event_source.get_new_events(
                 user=U_APPLE,
                 from_key=0,
-                limit=None,
+                limit=0,
                 room_ids=[ROOM_ID],
                 is_guest=False,
             )
@@ -417,7 +417,7 @@ def test_typing_timeout(self) -> None:
             self.event_source.get_new_events(
                 user=U_APPLE,
                 from_key=1,
-                limit=None,
+                limit=0,
                 room_ids=[ROOM_ID],
                 is_guest=False,
             )
@@ -452,7 +452,7 @@ def test_typing_timeout(self) -> None:
             self.event_source.get_new_events(
                 user=U_APPLE,
                 from_key=0,
-                limit=None,
+                limit=0,
                 room_ids=[ROOM_ID],
                 is_guest=False,
             )

From 65e1fb707bc926e0e8d0a77c072ddd0aac4a384d Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 10:18:39 +0000
Subject: [PATCH 03/10] Convince mypy that we are the main process

The get_typing_handler hs method can return different types
depending on whether you are the main process or a worker.

We make use of methods that are specific to the main process variant,
so perform a type-cast to make mypy happy.
---
 synapse/server.py             | 14 ++++++++++++--
 tests/handlers/test_typing.py | 11 +++++++++--
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/synapse/server.py b/synapse/server.py
index f4ab94c4f33c..ed12ecfb86b2 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -21,7 +21,17 @@
 import abc
 import functools
 import logging
-from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, TypeVar, cast
+from typing import (
+    TYPE_CHECKING,
+    Any,
+    Callable,
+    Dict,
+    List,
+    Optional,
+    TypeVar,
+    Union,
+    cast,
+)
 
 from twisted.internet.interfaces import IOpenSSLContextFactory
 from twisted.internet.tcp import Port
@@ -479,7 +489,7 @@ def get_presence_router(self) -> PresenceRouter:
         return PresenceRouter(self)
 
     @cache_in_self
-    def get_typing_handler(self) -> FollowerTypingHandler:
+    def get_typing_handler(self) -> Union[TypingWriterHandler, FollowerTypingHandler]:
         if self.get_instance_name() in self.config.worker.writers.typing:
             # Use get_typing_writer_handler to ensure that we use the same
             # cached version.
diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 53deecacdf33..4f6c8f6d1c2e 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -14,7 +14,7 @@
 
 
 import json
-from typing import Dict
+from typing import Dict, List, Set, cast
 from unittest.mock import ANY, Mock, call
 
 from twisted.internet import defer
@@ -24,6 +24,7 @@
 from synapse.api.constants import EduTypes
 from synapse.api.errors import AuthError
 from synapse.federation.transport.server import TransportLayerServer
+from synapse.handlers.typing import TypingWriterHandler
 from synapse.server import HomeServer
 from synapse.types import JsonDict, Requester, UserID, create_requester
 from synapse.util import Clock
@@ -98,7 +99,13 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         mock_notifier = hs.get_notifier()
         self.on_new_event = mock_notifier.on_new_event
 
-        self.handler = hs.get_typing_handler()
+        self.handler = cast(TypingWriterHandler, hs.get_typing_handler())
+
+        # hs.get_typing_handler will return a TypingWriterHandler when calling it
+        # from the main process, and a FollowerTypingHandler on workers.
+        # We rely on methods only available on the former, so assert we have the
+        # correct type here.
+        self.assertIsInstance(self.handler, TypingWriterHandler)
 
         self.event_source = hs.get_event_sources().sources.typing
 

From fd894ae4bf31d3e6ac396d50491dfcfe34115875 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 10:20:49 +0000
Subject: [PATCH 04/10] Ignore type assignments for mocked methods

mypy doesn't appear to have a better way to handle this.
https://github.com/python/mypy/issues/2427

Convert method assignments to Mock usages were possible.
---
 tests/handlers/test_typing.py | 60 +++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 4f6c8f6d1c2e..746a6be06155 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -17,7 +17,6 @@
 from typing import Dict, List, Set, cast
 from unittest.mock import ANY, Mock, call
 
-from twisted.internet import defer
 from twisted.test.proto_helpers import MemoryReactor
 from twisted.web.resource import Resource
 
@@ -110,24 +109,24 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.event_source = hs.get_event_sources().sources.typing
 
         self.datastore = hs.get_datastores().main
+
         self.datastore.get_destination_retry_timings = Mock(
             return_value=make_awaitable(None)
         )
 
-        self.datastore.get_device_updates_by_remote = Mock(
+        self.datastore.get_device_updates_by_remote = Mock(  # type: ignore[assignment]
             return_value=make_awaitable((0, []))
         )
 
-        self.datastore.get_destination_last_successful_stream_ordering = Mock(
+        self.datastore.get_destination_last_successful_stream_ordering = Mock(  # type: ignore[assignment]
             return_value=make_awaitable(None)
         )
 
-        def get_received_txn_response(*args):
-            return defer.succeed(None)
-
-        self.datastore.get_received_txn_response = get_received_txn_response
+        self.datastore.get_received_txn_response = Mock(  # type: ignore[assignment]
+            return_value=make_awaitable(None)
+        )
 
-        self.room_members = []
+        self.room_members: List[UserID] = []
 
         async def check_user_in_room(room_id: str, requester: Requester) -> None:
             if requester.user.to_string() not in [
@@ -136,47 +135,54 @@ async def check_user_in_room(room_id: str, requester: Requester) -> None:
                 raise AuthError(401, "User is not in the room")
             return None
 
-        hs.get_auth().check_user_in_room = check_user_in_room
+        hs.get_auth().check_user_in_room = Mock(  # type: ignore[assignment]
+            side_effect=check_user_in_room
+        )
 
         async def check_host_in_room(room_id: str, server_name: str) -> bool:
             return room_id == ROOM_ID
 
-        hs.get_event_auth_handler().is_host_in_room = check_host_in_room
+        hs.get_event_auth_handler().is_host_in_room = Mock(  # type: ignore[assignment]
+            side_effect=check_host_in_room
+        )
 
-        async def get_current_hosts_in_room(room_id: str):
+        async def get_current_hosts_in_room(room_id: str) -> Set[str]:
             return {member.domain for member in self.room_members}
 
-        hs.get_storage_controllers().state.get_current_hosts_in_room = (
-            get_current_hosts_in_room
+        hs.get_storage_controllers().state.get_current_hosts_in_room = Mock(  # type: ignore[assignment]
+            side_effect=get_current_hosts_in_room
         )
 
-        hs.get_storage_controllers().state.get_current_hosts_in_room_or_partial_state_approximation = (
-            get_current_hosts_in_room
+        hs.get_storage_controllers().state.get_current_hosts_in_room_or_partial_state_approximation = Mock(  # type: ignore[assignment]
+            side_effect=get_current_hosts_in_room
         )
 
-        async def get_users_in_room(room_id: str):
+        async def get_users_in_room(room_id: str) -> Set[str]:
             return {str(u) for u in self.room_members}
 
-        self.datastore.get_users_in_room = get_users_in_room
+        self.datastore.get_users_in_room = Mock(side_effect=get_users_in_room)
 
-        self.datastore.get_user_directory_stream_pos = Mock(
+        self.datastore.get_user_directory_stream_pos = Mock(  # type: ignore[assignment]
             side_effect=(
-                # we deliberately return a non-None stream pos to avoid doing an initial_spam
+                # we deliberately return a non-None stream pos to avoid
+                # doing an initial_sync
                 lambda: make_awaitable(1)
             )
         )
 
-        self.datastore.get_partial_current_state_deltas = Mock(return_value=(0, None))
+        self.datastore.get_partial_current_state_deltas = Mock(return_value=(0, None))  # type: ignore[assignment]
 
-        self.datastore.get_to_device_stream_token = lambda: 0
-        self.datastore.get_new_device_msgs_for_remote = (
-            lambda *args, **kargs: make_awaitable(([], 0))
+        self.datastore.get_to_device_stream_token = Mock(  # type: ignore[assignment]
+            side_effect=lambda: 0
+        )
+        self.datastore.get_new_device_msgs_for_remote = Mock(  # type: ignore[assignment]
+            side_effect=lambda *args, **kargs: make_awaitable(([], 0))
         )
-        self.datastore.delete_device_msgs_for_remote = (
-            lambda *args, **kargs: make_awaitable(None)
+        self.datastore.delete_device_msgs_for_remote = Mock(  # type: ignore[assignment]
+            side_effect=lambda *args, **kargs: make_awaitable(None)
         )
-        self.datastore.set_received_txn_response = (
-            lambda *args, **kwargs: make_awaitable(None)
+        self.datastore.set_received_txn_response = Mock(  # type: ignore[assignment]
+            side_effect=lambda *args, **kwargs: make_awaitable(None)
         )
 
     def test_started_typing_local(self) -> None:

From 9898966e64a0cfba50c2c13c8b469167b763a276 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 10:24:59 +0000
Subject: [PATCH 05/10] Fix typing of a mocked hs attribute

mypy thought self.mocked_notifier was an actual Notifier due to the
type signature of HomeServer.get_notifier.

By avoiding that method, mypy figures out what we're trying to do.
---
 tests/handlers/test_typing.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 746a6be06155..e2002715d2d9 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -80,8 +80,9 @@ def make_homeserver(
         # the tests assume that we are starting at unix time 1000
         reactor.pump((1000,))
 
+        self.mock_hs_notifier = Mock()
         hs = self.setup_test_homeserver(
-            notifier=Mock(),
+            notifier=self.mock_hs_notifier,
             federation_http_client=mock_federation_client,
             keyring=mock_keyring,
             replication_streams={},
@@ -95,8 +96,7 @@ def create_resource_dict(self) -> Dict[str, Resource]:
         return d
 
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
-        mock_notifier = hs.get_notifier()
-        self.on_new_event = mock_notifier.on_new_event
+        self.on_new_event = self.mock_hs_notifier.on_new_event
 
         self.handler = cast(TypingWriterHandler, hs.get_typing_handler())
 

From 39d56eb6809a8aad7f3d2dec15053a1e888a5383 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 10:25:29 +0000
Subject: [PATCH 06/10] Remove tests/handlers/test_typing.py from the mypy
 exclude list

---
 mypy.ini | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mypy.ini b/mypy.ini
index 468bfe588ccc..f3fbbad75423 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -41,7 +41,6 @@ exclude = (?x)
    |tests/federation/test_federation_catch_up.py
    |tests/federation/test_federation_sender.py
    |tests/federation/transport/test_knocking.py
-   |tests/handlers/test_typing.py
    |tests/http/federation/test_matrix_federation_agent.py
    |tests/http/federation/test_srv_resolver.py
    |tests/http/test_proxyagent.py

From a55bda87d853512220ce171fe7855664131855da Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sat, 21 Jan 2023 13:07:20 +0000
Subject: [PATCH 07/10] changelog

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

diff --git a/changelog.d/14886.misc b/changelog.d/14886.misc
new file mode 100644
index 000000000000..9f5384e60e78
--- /dev/null
+++ b/changelog.d/14886.misc
@@ -0,0 +1 @@
+Add missing type hints.
\ No newline at end of file

From ccd5790136a635dc959254c00888a0d6709f4bfa Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sun, 22 Jan 2023 11:07:48 +0100
Subject: [PATCH 08/10] Remove unnecessary union

---
 synapse/server.py | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/synapse/server.py b/synapse/server.py
index ed12ecfb86b2..f4ab94c4f33c 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -21,17 +21,7 @@
 import abc
 import functools
 import logging
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    Callable,
-    Dict,
-    List,
-    Optional,
-    TypeVar,
-    Union,
-    cast,
-)
+from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, TypeVar, cast
 
 from twisted.internet.interfaces import IOpenSSLContextFactory
 from twisted.internet.tcp import Port
@@ -489,7 +479,7 @@ def get_presence_router(self) -> PresenceRouter:
         return PresenceRouter(self)
 
     @cache_in_self
-    def get_typing_handler(self) -> Union[TypingWriterHandler, FollowerTypingHandler]:
+    def get_typing_handler(self) -> FollowerTypingHandler:
         if self.get_instance_name() in self.config.worker.writers.typing:
             # Use get_typing_writer_handler to ensure that we use the same
             # cached version.

From 0a00bbabe56007e282758a4520372091cd6b8340 Mon Sep 17 00:00:00 2001
From: Andrew Morgan <andrew@amorgan.xyz>
Date: Sun, 22 Jan 2023 11:10:25 +0100
Subject: [PATCH 09/10] use a bare assert

---
 tests/handlers/test_typing.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index e2002715d2d9..7e6bf03847c7 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -98,13 +98,14 @@ def create_resource_dict(self) -> Dict[str, Resource]:
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.on_new_event = self.mock_hs_notifier.on_new_event
 
-        self.handler = cast(TypingWriterHandler, hs.get_typing_handler())
-
         # hs.get_typing_handler will return a TypingWriterHandler when calling it
         # from the main process, and a FollowerTypingHandler on workers.
         # We rely on methods only available on the former, so assert we have the
-        # correct type here.
-        self.assertIsInstance(self.handler, TypingWriterHandler)
+        # correct type here. We have to assign self.handler after the assert,
+        # otherwise mypy will treat it as a FollowerTypingHandler
+        handler = hs.get_typing_handler()
+        assert isinstance(handler, TypingWriterHandler)
+        self.handler = handler
 
         self.event_source = hs.get_event_sources().sources.typing
 

From 94a2d91763495c6c8ba33087e62b6019f4fd7d18 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Wed, 25 Jan 2023 15:14:56 -0500
Subject: [PATCH 10/10] Lint.

---
 tests/handlers/test_typing.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py
index 7e6bf03847c7..1fe9563c98f3 100644
--- a/tests/handlers/test_typing.py
+++ b/tests/handlers/test_typing.py
@@ -14,7 +14,7 @@
 
 
 import json
-from typing import Dict, List, Set, cast
+from typing import Dict, List, Set
 from unittest.mock import ANY, Mock, call
 
 from twisted.test.proto_helpers import MemoryReactor