From e3b807408fc1b252aed1007d0b872a48eedbb029 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sun, 15 Aug 2021 22:42:13 +0200
Subject: [PATCH 1/8] Type hints for proxyagent

---
 mypy.ini                   | 2 ++
 synapse/http/proxyagent.py | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/mypy.ini b/mypy.ini
index 5d6cd557bca2..005078dcdd16 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -28,10 +28,12 @@ files =
   synapse/federation,
   synapse/groups,
   synapse/handlers,
+  synapse/http/additional_resource.py,
   synapse/http/client.py,
   synapse/http/federation/matrix_federation_agent.py,
   synapse/http/federation/well_known_resolver.py,
   synapse/http/matrixfederationclient.py,
+  synapse/http/proxyagent.py,
   synapse/http/servlet.py,
   synapse/http/server.py,
   synapse/http/site.py,
diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py
index a3f31452d0cc..c7717c2b45ed 100644
--- a/synapse/http/proxyagent.py
+++ b/synapse/http/proxyagent.py
@@ -173,7 +173,9 @@ def request(
             raise ValueError(f"Invalid URI {uri!r}")
 
         parsed_uri = URI.fromBytes(uri)
-        pool_key = (parsed_uri.scheme, parsed_uri.host, parsed_uri.port)
+        pool_key = (
+            f"{parsed_uri.scheme.decode()}{parsed_uri.host.decode()}{parsed_uri.port}"
+        )
         request_path = parsed_uri.originForm
 
         should_skip_proxy = False
@@ -198,8 +200,9 @@ def request(
                     self.http_proxy_creds.as_proxy_authorization_value(),
                 )
             # Cache *all* connections under the same key, since we are only
-            # connecting to a single destination, the proxy:
-            pool_key = ("http-proxy", self.http_proxy_endpoint)
+            # connecting to a single destination, the proxy
+            # The URL of proxy is not important for the key
+            pool_key = "http-proxy"
             endpoint = self.http_proxy_endpoint
             request_path = uri
         elif (

From f43439ebfe6591d73a3df414a9e99ca6ce246850 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sun, 15 Aug 2021 22:49:23 +0200
Subject: [PATCH 2/8] newsfile

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

diff --git a/changelog.d/10608.misc b/changelog.d/10608.misc
new file mode 100644
index 000000000000..8d40b29e6e55
--- /dev/null
+++ b/changelog.d/10608.misc
@@ -0,0 +1 @@
+Type hints for proxyagent.
\ No newline at end of file

From 0a5b8e385fddd18d241e29b2edc2419248603195 Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Sun, 15 Aug 2021 23:02:14 +0200
Subject: [PATCH 3/8] small fix

---
 synapse/http/proxyagent.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py
index c7717c2b45ed..cf8cd05c7c20 100644
--- a/synapse/http/proxyagent.py
+++ b/synapse/http/proxyagent.py
@@ -173,9 +173,7 @@ def request(
             raise ValueError(f"Invalid URI {uri!r}")
 
         parsed_uri = URI.fromBytes(uri)
-        pool_key = (
-            f"{parsed_uri.scheme.decode()}{parsed_uri.host.decode()}{parsed_uri.port}"
-        )
+        pool_key = f"{parsed_uri.scheme!r}{parsed_uri.host!r}{parsed_uri.port}"
         request_path = parsed_uri.originForm
 
         should_skip_proxy = False

From 82328351ef74bed240eff8086e4a9772158cf3ff Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 16 Aug 2021 00:20:27 +0200
Subject: [PATCH 4/8] add srv_resolver

---
 changelog.d/10608.misc                  |  2 +-
 mypy.ini                                |  1 +
 synapse/http/federation/srv_resolver.py | 41 ++++++++++++++-----------
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/changelog.d/10608.misc b/changelog.d/10608.misc
index 8d40b29e6e55..f25d994e52c1 100644
--- a/changelog.d/10608.misc
+++ b/changelog.d/10608.misc
@@ -1 +1 @@
-Type hints for proxyagent.
\ No newline at end of file
+Type hints for proxyagent and srv_resolver.
\ No newline at end of file
diff --git a/mypy.ini b/mypy.ini
index 005078dcdd16..41b78095fcfc 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -31,6 +31,7 @@ files =
   synapse/http/additional_resource.py,
   synapse/http/client.py,
   synapse/http/federation/matrix_federation_agent.py,
+  synapse/http/federation/srv_resolver.py,
   synapse/http/federation/well_known_resolver.py,
   synapse/http/matrixfederationclient.py,
   synapse/http/proxyagent.py,
diff --git a/synapse/http/federation/srv_resolver.py b/synapse/http/federation/srv_resolver.py
index b8ed4ec905d4..0c1461f83fde 100644
--- a/synapse/http/federation/srv_resolver.py
+++ b/synapse/http/federation/srv_resolver.py
@@ -16,7 +16,7 @@
 import logging
 import random
 import time
-from typing import List
+from typing import Callable, Dict, List
 
 import attr
 
@@ -28,7 +28,7 @@
 
 logger = logging.getLogger(__name__)
 
-SERVER_CACHE = {}
+SERVER_CACHE: Dict = {}
 
 
 @attr.s(slots=True, frozen=True)
@@ -37,26 +37,26 @@ class Server:
     Our record of an individual server which can be tried to reach a destination.
 
     Attributes:
-        host (bytes): target hostname
-        port (int):
-        priority (int):
-        weight (int):
-        expires (int): when the cache should expire this record - in *seconds* since
+        host: target hostname
+        port:
+        priority:
+        weight:
+        expires: when the cache should expire this record - in *seconds* since
             the epoch
     """
 
-    host = attr.ib()
-    port = attr.ib()
-    priority = attr.ib(default=0)
-    weight = attr.ib(default=0)
-    expires = attr.ib(default=0)
+    host: bytes = attr.ib()
+    port: int = attr.ib()
+    priority: int = attr.ib(default=0)
+    weight: int = attr.ib(default=0)
+    expires: int = attr.ib(default=0)
 
 
 def _sort_server_list(server_list):
     """Given a list of SRV records sort them into priority order and shuffle
     each priority with the given weight.
     """
-    priority_map = {}
+    priority_map: Dict[int, List[Server]] = {}
 
     for server in server_list:
         priority_map.setdefault(server.priority, []).append(server)
@@ -103,11 +103,16 @@ class SrvResolver:
 
     Args:
         dns_client (twisted.internet.interfaces.IResolver): twisted resolver impl
-        cache (dict): cache object
-        get_time (callable): clock implementation. Should return seconds since the epoch
+        cache: cache object
+        get_time: clock implementation. Should return seconds since the epoch
     """
 
-    def __init__(self, dns_client=client, cache=SERVER_CACHE, get_time=time.time):
+    def __init__(
+        self,
+        dns_client=client,
+        cache: Dict = SERVER_CACHE,
+        get_time: Callable = time.time,
+    ):
         self._dns_client = dns_client
         self._cache = cache
         self._get_time = get_time
@@ -116,7 +121,7 @@ async def resolve_service(self, service_name: bytes) -> List[Server]:
         """Look up a SRV record
 
         Args:
-            service_name (bytes): record to look up
+            service_name: record to look up
 
         Returns:
             a list of the SRV records, or an empty list if none found
@@ -158,7 +163,7 @@ async def resolve_service(self, service_name: bytes) -> List[Server]:
             and answers[0].payload
             and answers[0].payload.target == dns.Name(b".")
         ):
-            raise ConnectError("Service %s unavailable" % service_name)
+            raise ConnectError(f"Service {service_name!r} unavailable")
 
         servers = []
 

From 0c3d2c0f22ae38f4cb623a624b8301a35071205d Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 16 Aug 2021 22:29:38 +0200
Subject: [PATCH 5/8] update hints and comment

---
 synapse/http/additional_resource.py     | 15 +++++++++++----
 synapse/http/federation/srv_resolver.py | 18 +++++++++---------
 synapse/http/proxyagent.py              |  3 +--
 3 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/synapse/http/additional_resource.py b/synapse/http/additional_resource.py
index 55ea97a07f29..bf9069af5bc5 100644
--- a/synapse/http/additional_resource.py
+++ b/synapse/http/additional_resource.py
@@ -12,8 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from typing import TYPE_CHECKING
+
+from twisted.web.server import Request
+
 from synapse.http.server import DirectServeJsonResource
 
+if TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 
 class AdditionalResource(DirectServeJsonResource):
     """Resource wrapper for additional_resources
@@ -25,7 +32,7 @@ class AdditionalResource(DirectServeJsonResource):
     and exception handling.
     """
 
-    def __init__(self, hs, handler):
+    def __init__(self, hs: "HomeServer", handler: Request):
         """Initialise AdditionalResource
 
         The ``handler`` should return a deferred which completes when it has
@@ -33,14 +40,14 @@ def __init__(self, hs, handler):
         ``request.write()``, and call ``request.finish()``.
 
         Args:
-            hs (synapse.server.HomeServer): homeserver
-            handler ((twisted.web.server.Request) -> twisted.internet.defer.Deferred):
+            hs: homeserver
+            handler (twisted.internet.defer.Deferred):
                 function to be called to handle the request.
         """
         super().__init__()
         self._handler = handler
 
-    def _async_render(self, request):
+    def _async_render(self, request: Request):
         # Cheekily pass the result straight through, so we don't need to worry
         # if its an awaitable or not.
         return self._handler(request)
diff --git a/synapse/http/federation/srv_resolver.py b/synapse/http/federation/srv_resolver.py
index 0c1461f83fde..6a9ce33d4ebf 100644
--- a/synapse/http/federation/srv_resolver.py
+++ b/synapse/http/federation/srv_resolver.py
@@ -28,10 +28,10 @@
 
 logger = logging.getLogger(__name__)
 
-SERVER_CACHE: Dict = {}
+SERVER_CACHE: Dict[bytes, List[Server]] = {}
 
 
-@attr.s(slots=True, frozen=True)
+@attr.s(auto_attribs=True, slots=True, frozen=True)
 class Server:
     """
     Our record of an individual server which can be tried to reach a destination.
@@ -45,11 +45,11 @@ class Server:
             the epoch
     """
 
-    host: bytes = attr.ib()
-    port: int = attr.ib()
-    priority: int = attr.ib(default=0)
-    weight: int = attr.ib(default=0)
-    expires: int = attr.ib(default=0)
+    host: bytes
+    port: int
+    priority: int = 0
+    weight: int = 0
+    expires: int = 0
 
 
 def _sort_server_list(server_list):
@@ -110,8 +110,8 @@ class SrvResolver:
     def __init__(
         self,
         dns_client=client,
-        cache: Dict = SERVER_CACHE,
-        get_time: Callable = time.time,
+        cache: Dict[bytes, List[Server]] = SERVER_CACHE,
+        get_time: Callable[[], float] = time.time,
     ):
         self._dns_client = dns_client
         self._cache = cache
diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py
index cf8cd05c7c20..6fd88bde204c 100644
--- a/synapse/http/proxyagent.py
+++ b/synapse/http/proxyagent.py
@@ -198,8 +198,7 @@ def request(
                     self.http_proxy_creds.as_proxy_authorization_value(),
                 )
             # Cache *all* connections under the same key, since we are only
-            # connecting to a single destination, the proxy
-            # The URL of proxy is not important for the key
+            # connecting to a single destination, the proxy:
             pool_key = "http-proxy"
             endpoint = self.http_proxy_endpoint
             request_path = uri

From f596719356409dfc4b894bf10abd9fe682afc77b Mon Sep 17 00:00:00 2001
From: dklimpel <5740567+dklimpel@users.noreply.github.com>
Date: Mon, 16 Aug 2021 23:38:17 +0200
Subject: [PATCH 6/8] fix CI

---
 synapse/http/additional_resource.py     | 4 ++--
 synapse/http/federation/srv_resolver.py | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/synapse/http/additional_resource.py b/synapse/http/additional_resource.py
index bf9069af5bc5..9a2684aca432 100644
--- a/synapse/http/additional_resource.py
+++ b/synapse/http/additional_resource.py
@@ -32,7 +32,7 @@ class AdditionalResource(DirectServeJsonResource):
     and exception handling.
     """
 
-    def __init__(self, hs: "HomeServer", handler: Request):
+    def __init__(self, hs: "HomeServer", handler):
         """Initialise AdditionalResource
 
         The ``handler`` should return a deferred which completes when it has
@@ -41,7 +41,7 @@ def __init__(self, hs: "HomeServer", handler: Request):
 
         Args:
             hs: homeserver
-            handler (twisted.internet.defer.Deferred):
+            handler ((twisted.web.server.Request) -> twisted.internet.defer.Deferred):
                 function to be called to handle the request.
         """
         super().__init__()
diff --git a/synapse/http/federation/srv_resolver.py b/synapse/http/federation/srv_resolver.py
index 6a9ce33d4ebf..eeb1a97dcdf0 100644
--- a/synapse/http/federation/srv_resolver.py
+++ b/synapse/http/federation/srv_resolver.py
@@ -28,7 +28,7 @@
 
 logger = logging.getLogger(__name__)
 
-SERVER_CACHE: Dict[bytes, List[Server]] = {}
+SERVER_CACHE: Dict[bytes, List] = {}
 
 
 @attr.s(auto_attribs=True, slots=True, frozen=True)

From d1cc3fbb331316ee044b2153e8cc181c2b464a84 Mon Sep 17 00:00:00 2001
From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
Date: Wed, 18 Aug 2021 18:42:32 +0200
Subject: [PATCH 7/8] Apply suggestions from code review

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
---
 synapse/http/federation/srv_resolver.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/synapse/http/federation/srv_resolver.py b/synapse/http/federation/srv_resolver.py
index eeb1a97dcdf0..f68646fd0dd4 100644
--- a/synapse/http/federation/srv_resolver.py
+++ b/synapse/http/federation/srv_resolver.py
@@ -28,7 +28,7 @@
 
 logger = logging.getLogger(__name__)
 
-SERVER_CACHE: Dict[bytes, List] = {}
+SERVER_CACHE: Dict[bytes, List["Server"]] = {}
 
 
 @attr.s(auto_attribs=True, slots=True, frozen=True)
@@ -52,7 +52,7 @@ class Server:
     expires: int = 0
 
 
-def _sort_server_list(server_list):
+def _sort_server_list(server_list: List[Server]) -> List[Server]:
     """Given a list of SRV records sort them into priority order and shuffle
     each priority with the given weight.
     """

From e7c5cd33a5b268fae9e440d5979869d23dbb7749 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Wed, 18 Aug 2021 13:17:34 -0400
Subject: [PATCH 8/8] Update changelog.

---
 changelog.d/10608.misc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/10608.misc b/changelog.d/10608.misc
index f25d994e52c1..875bdd2fd02d 100644
--- a/changelog.d/10608.misc
+++ b/changelog.d/10608.misc
@@ -1 +1 @@
-Type hints for proxyagent and srv_resolver.
\ No newline at end of file
+Improve type hints for the proxy agent and SRV resolver modules. Contributed by @dklimpel.
\ No newline at end of file