Browse Source

`IAgent`s should return `IResponse`s (#443)

I tried to use `Response` rather than `IResponse` to avoid having to
write a stub for the latter. There were other complications down the
line from this. It was easier to change my mind, use `stubgen` and
polish up the `iweb` stub.

See also https://github.com/matrix-org/sydent/pull/439#issuecomment-952954253
David Robertson 2 years ago
parent
commit
48d2e7fb04

+ 1 - 0
changelog.d/443.misc

@@ -0,0 +1 @@
+More accurately use `IResponse` rather than `http.Response` in type hints.

+ 1 - 1
stubs/twisted/internet/ssl.pyi

@@ -36,7 +36,7 @@ def optionsForClientTLS(
     extraCertificateOptions: Optional[Dict[Any, Any]] = None,
 ) -> IOpenSSLClientConnectionCreator: ...
 
-# Type safety: I don't want to respecify the methods on the interface that we
+# Type ignore: I don't want to respecify the methods on the interface that we
 # don't use.
 @implementer(IOpenSSLTrustRoot)  # type: ignore[misc]
 class OpenSSLDefaultPaths: ...

+ 31 - 6
stubs/twisted/web/client.pyi

@@ -1,4 +1,4 @@
-from typing import Any, BinaryIO, Optional
+from typing import Any, BinaryIO, Optional, Type, TypeVar
 
 import twisted.internet
 from twisted.internet.defer import Deferred
@@ -12,6 +12,8 @@ from twisted.web.http_headers import Headers
 from twisted.web.iweb import IAgent, IBodyProducer, IPolicyForHTTPS, IResponse
 from zope.interface import implementer
 
+C = TypeVar("C")
+
 @implementer(IPolicyForHTTPS)
 class BrowserLikePolicyForHTTPS:
     def creatorForNetloc(
@@ -30,16 +32,13 @@ class Agent:
         bindAddress: Optional[bytes] = None,
         pool: Optional[HTTPConnectionPool] = None,
     ): ...
-    # Type safety: IAgent says this returns a Deferred[IResponse].
-    # I'm narrowing it here (but that's not strictly allowed because Deferred[T]
-    # is _contra_variant in T. It's all muddling.
-    def request(  # type: ignore[override]
+    def request(
         self,
         method: bytes,
         uri: bytes,
         headers: Optional[Headers] = None,
         bodyProducer: Optional[IBodyProducer] = None,
-    ) -> Deferred[Response]: ...
+    ) -> Deferred[IResponse]: ...
 
 @implementer(IBodyProducer)
 class FileBodyProducer:
@@ -62,6 +61,9 @@ class FileBodyProducer:
 
 def readBody(response: IResponse) -> Deferred[bytes]: ...
 
+# Type ignore: I don't want to respecify the methods on the interface that we
+# don't use.
+@implementer(IResponse)  # type: ignore[misc]
 class Response:
     code: int
     headers: Headers
@@ -70,3 +72,26 @@ class Response:
     def deliverBody(self, protocol: IProtocol) -> None: ...
 
 class ResponseDone: ...
+
+class URI:
+    scheme: bytes
+    netloc: bytes
+    host: bytes
+    port: int
+    path: bytes
+    params: bytes
+    query: bytes
+    fragment: bytes
+    def __init__(
+        self,
+        scheme: bytes,
+        netloc: bytes,
+        host: bytes,
+        port: int,
+        path: bytes,
+        params: bytes,
+        query: bytes,
+        fragment: bytes,
+    ): ...
+    @classmethod
+    def fromBytes(cls: Type[C], uri: bytes, defaultPort: Optional[int] = None) -> C: ...

+ 7 - 0
stubs/twisted/web/http.pyi

@@ -5,9 +5,14 @@ from twisted.internet.defer import Deferred
 from twisted.internet.interfaces import IAddress, ITCPTransport
 from twisted.logger import Logger
 from twisted.web.http_headers import Headers
+from twisted.web.iweb import IRequest
+from zope.interface import implementer
 
 class HTTPChannel: ...
 
+# Type ignore: I don't want to respecify the methods on the interface that we
+# don't use.
+@implementer(IRequest)  # type: ignore[misc]
 class Request:
     # Instance attributes mentioned in the docstring
     method: bytes
@@ -43,3 +48,5 @@ class Request:
     def handleContentChunk(self, data: bytes) -> None: ...
 
 class PotentialDataLoss(Exception): ...
+
+CACHED: object

+ 103 - 0
stubs/twisted/web/iweb.pyi

@@ -0,0 +1,103 @@
+from typing import Any, AnyStr, BinaryIO, Dict, List, Mapping, Optional, Tuple
+
+from twisted.cred.credentials import IUsernameDigestHash as IUsernameDigestHash
+from twisted.internet.defer import Deferred
+from twisted.internet.interfaces import (
+    IAddress,
+    IConsumer,
+    IOpenSSLClientConnectionCreator,
+    IProtocol,
+    IPushProducer,
+    IStreamClientEndpoint,
+)
+from twisted.python.urlpath import URLPath
+from twisted.web.client import URI
+from twisted.web.http_headers import Headers
+from typing_extensions import Literal
+from zope.interface import Interface
+
+class IClientRequest(Interface):
+    method: bytes
+    absoluteURI: Optional[bytes]
+    headers: Headers
+
+class IRequest(Interface):
+    method: bytes
+    uri: bytes
+    path: bytes
+    args: Mapping[bytes, List[bytes]]
+    prepath: List[bytes]
+    postpath: List[bytes]
+    requestHeaders: Headers
+    content: BinaryIO
+    responseHeaders: Headers
+    def getHeader(key: AnyStr) -> Optional[AnyStr]: ...
+    def getCookie(key: bytes) -> Optional[bytes]: ...
+    def getAllHeaders() -> Dict[bytes, bytes]: ...
+    def getRequestHostname() -> bytes: ...
+    def getHost() -> IAddress: ...
+    def getClientAddress() -> IAddress: ...
+    def getClientIP() -> Optional[str]: ...
+    def getUser() -> str: ...
+    def getPassword() -> str: ...
+    def isSecure() -> bool: ...
+    def getSession(sessionInterface: Any | None = ...) -> Any: ...
+    def URLPath() -> URLPath: ...
+    def prePathURL() -> bytes: ...
+    def rememberRootURL() -> None: ...
+    def getRootURL() -> bytes: ...
+    def finish() -> None: ...
+    def write(data: bytes) -> None: ...
+    def addCookie(
+        k: AnyStr,
+        v: AnyStr,
+        expires: Optional[AnyStr] = ...,
+        domain: Optional[AnyStr] = ...,
+        path: Optional[AnyStr] = ...,
+        max_age: Optional[AnyStr] = ...,
+        comment: Optional[AnyStr] = ...,
+        secure: Optional[bool] = ...,
+    ) -> None: ...
+    def setResponseCode(code: int, message: Optional[bytes] = ...) -> None: ...
+    def setHeader(k: AnyStr, v: AnyStr) -> None: ...
+    def redirect(url: AnyStr) -> None: ...
+    # returns http.CACHED or False. http.CACHED is a string constant, but we
+    # treat it as an opaque object, similar to UNKNOWN_LENGTH.
+    def setLastModified(when: float) -> object | Literal[False]: ...
+    def setETag(etag: str) -> object | Literal[False]: ...
+    def setHost(host: bytes, port: int, ssl: bool = ...) -> None: ...
+
+class IBodyProducer(IPushProducer):
+    # Length is either `int` or the opaque object UNKNOWN_LENGTH.
+    length: int | object
+    def startProducing(consumer: IConsumer) -> Deferred[None]: ...
+    def stopProducing() -> None: ...
+
+class IResponse(Interface):
+    version: Tuple[str, int, int]
+    code: int
+    phrase: str
+    headers: Headers
+    length: int | object
+    request: IClientRequest
+    previousResponse: Optional[IResponse]
+    def deliverBody(protocol: IProtocol) -> None: ...
+    def setPreviousResponse(response: IResponse) -> None: ...
+
+class IAgent(Interface):
+    def request(
+        method: bytes,
+        uri: bytes,
+        headers: Optional[Headers] = ...,
+        bodyProducer: Optional[IBodyProducer] = ...,
+    ) -> Deferred[IResponse]: ...
+
+class IPolicyForHTTPS(Interface):
+    def creatorForNetloc(
+        hostname: bytes, port: int
+    ) -> IOpenSSLClientConnectionCreator: ...
+
+class IAgentEndpointFactory(Interface):
+    def endpointForURI(uri: URI) -> IStreamClientEndpoint: ...
+
+UNKNOWN_LENGTH: object

+ 7 - 6
sydent/http/httpclient.py

@@ -17,8 +17,9 @@ import logging
 from io import BytesIO
 from typing import TYPE_CHECKING, Any, Dict, Generic, Optional, Tuple, TypeVar, cast
 
-from twisted.web.client import Agent, FileBodyProducer, Response
+from twisted.web.client import Agent, FileBodyProducer
 from twisted.web.http_headers import Headers
+from twisted.web.iweb import IAgent, IResponse
 
 from sydent.http.blacklisting_reactor import BlacklistingReactorWrapper
 from sydent.http.federation_tls_options import ClientTLSOptionsFactory
@@ -33,7 +34,7 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
-AgentType = TypeVar("AgentType", Agent, MatrixFederationAgent)
+AgentType = TypeVar("AgentType", bound=IAgent)
 
 
 class HTTPClient(Generic[AgentType]):
@@ -54,7 +55,7 @@ class HTTPClient(Generic[AgentType]):
         """
         logger.debug("HTTP GET %s", uri)
 
-        response: Response = await self.agent.request(
+        response = await self.agent.request(
             b"GET",
             uri.encode("utf8"),
         )
@@ -73,7 +74,7 @@ class HTTPClient(Generic[AgentType]):
 
     async def post_json_get_nothing(
         self, uri: str, post_json: JsonDict, opts: Dict[str, Any]
-    ) -> Response:
+    ) -> IResponse:
         """Make a POST request to an endpoint returning nothing
 
         :param uri: The URI to make a POST request to.
@@ -95,7 +96,7 @@ class HTTPClient(Generic[AgentType]):
         post_json: Dict[str, Any],
         opts: Dict[str, Any],
         max_size: Optional[int] = None,
-    ) -> Tuple[Response, Optional[JsonDict]]:
+    ) -> Tuple[IResponse, Optional[JsonDict]]:
         """Make a POST request to an endpoint that might be returning JSON and parse
         result
 
@@ -125,7 +126,7 @@ class HTTPClient(Generic[AgentType]):
 
         logger.debug("HTTP POST %s -> %s", json_bytes, uri)
 
-        response: Response = await self.agent.request(
+        response = await self.agent.request(
             b"POST",
             uri.encode("utf8"),
             headers,

+ 3 - 3
sydent/http/httpcommon.py

@@ -23,9 +23,9 @@ from twisted.internet.interfaces import ITCPTransport
 from twisted.internet.protocol import connectionDone
 from twisted.python.failure import Failure
 from twisted.web import server
-from twisted.web.client import Response, ResponseDone
+from twisted.web.client import ResponseDone
 from twisted.web.http import PotentialDataLoss
-from twisted.web.iweb import UNKNOWN_LENGTH
+from twisted.web.iweb import UNKNOWN_LENGTH, IResponse
 
 if TYPE_CHECKING:
     from sydent.sydent import Sydent
@@ -166,7 +166,7 @@ class _ReadBodyWithMaxSizeProtocol(protocol.Protocol):
 
 
 def read_body_with_max_size(
-    response: Response, max_size: Optional[int]
+    response: IResponse, max_size: Optional[int]
 ) -> "defer.Deferred[bytes]":
     """
     Read a HTTP response body to a file-object. Optionally enforcing a maximum file size.