From 799b41fe47cfe2e56be33eee8cfbaf89a9853a8e Mon Sep 17 00:00:00 2001 From: Terence Honles Date: Fri, 10 Sep 2021 13:18:20 -0700 Subject: [PATCH] fix typing on HttpResponse and StreamingHttpResponse (#712) While the documentation for `HttpResponse` and `StreamingHttpResponse` *says* `content` and `streaming_content` should be bytestrings [1] or an iterable of bytestrings respectively [2], this is not what the API supports [3] [4] and there are tests which make sure the API supports more than bytestrings [5] [6] [etc]. Before assigning `content` or `streaming_content` the code paths will call `self.make_bytes` to coerce the value to bytes. [1]: https://github.com/django/django/blob/ecf87ad513fd8af6e4a6093ed918723a7d88d5ca/django/http/response.py#L324-L327 [2]: https://github.com/django/django/blob/0a28b42b1510b8093a90718bafd7627ed67fa13b/django/http/response.py#L395-L399 [3]: https://github.com/django/django/blob/ecf87ad513fd8af6e4a6093ed918723a7d88d5ca/django/http/response.py#L342-L362 [4]: https://github.com/django/django/blob/0a28b42b1510b8093a90718bafd7627ed67fa13b/django/http/response.py#L415-L427 [5]: https://github.com/django/django/blob/0a28b42b1510b8093a90718bafd7627ed67fa13b/tests/cache/tests.py#L2250 [6]: https://github.com/django/django/blob/0a28b42b1510b8093a90718bafd7627ed67fa13b/tests/i18n/urls.py#L8 --- django-stubs/http/response.pyi | 44 ++++++-- .../views/test_function_based_views.yml | 102 ++++++++++++++++++ 2 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 tests/typecheck/views/test_function_based_views.yml diff --git a/django-stubs/http/response.pyi b/django-stubs/http/response.pyi index ffa44e4..1efb369 100644 --- a/django-stubs/http/response.pyi +++ b/django-stubs/http/response.pyi @@ -1,7 +1,7 @@ import datetime from io import BytesIO from json import JSONEncoder -from typing import Any, Dict, Iterable, Iterator, List, Optional, Tuple, Type, Union, overload +from typing import Any, Dict, Generic, Iterable, Iterator, List, Optional, Tuple, Type, TypeVar, Union, overload from django.core.handlers.wsgi import WSGIRequest from django.http.cookie import SimpleCookie @@ -10,6 +10,31 @@ from django.test.client import Client from django.urls import ResolverMatch from django.utils.datastructures import CaseInsensitiveMapping +_T = TypeVar("_T") +_U = TypeVar("_U") + +class _PropertyDescriptor(Generic[_T, _U]): + """ + This helper property descriptor allows defining asynmetric getter/setters + which mypy currently doesn't support with either: + + class HttpResponse: + @property + def content(...): ... + @property.setter + def content(...): ... + + or: + + class HttpResponse: + def _get_content(...): ... + def _set_content(...): ... + content = property(_get_content, _set_content) + """ + + def __get__(self, instance: Any, owner: Optional[Any]) -> _U: ... + def __set__(self, instance: Any, value: _T) -> None: ... + class BadHeaderError(ValueError): ... class ResponseHeaders(CaseInsensitiveMapping): @@ -21,7 +46,7 @@ class ResponseHeaders(CaseInsensitiveMapping): def pop(self, key: str, default: Optional[str] = ...) -> str: ... def setdefault(self, key: str, value: str) -> None: ... -class HttpResponseBase(Iterable[Any]): +class HttpResponseBase: status_code: int = ... streaming: bool = ... cookies: SimpleCookie = ... @@ -72,10 +97,9 @@ class HttpResponseBase(Iterable[Any]): def seekable(self) -> bool: ... def writable(self) -> bool: ... def writelines(self, lines: Iterable[object]): ... - def __iter__(self) -> Iterator[Any]: ... -class HttpResponse(HttpResponseBase): - content: Any +class HttpResponse(HttpResponseBase, Iterable[bytes]): + content = _PropertyDescriptor[object, bytes]() csrf_cookie_set: bool redirect_chain: List[Tuple[str, int]] sameorigin: bool @@ -85,6 +109,7 @@ class HttpResponse(HttpResponseBase): def __init__(self, content: object = ..., *args: Any, **kwargs: Any) -> None: ... def serialize(self) -> bytes: ... __bytes__ = serialize + def __iter__(self) -> Iterator[bytes]: ... @property def url(self) -> str: ... # Attributes assigned by monkey-patching in test client ClientHandler.__call__() @@ -96,13 +121,12 @@ class HttpResponse(HttpResponseBase): context: Context resolver_match: ResolverMatch def json(self) -> Any: ... - def __iter__(self): ... def getvalue(self) -> bytes: ... -class StreamingHttpResponse(HttpResponseBase): - content: Any - streaming_content: Iterator[bytes] - def __init__(self, streaming_content: Iterable[bytes] = ..., *args: Any, **kwargs: Any) -> None: ... +class StreamingHttpResponse(HttpResponseBase, Iterable[bytes]): + streaming_content = _PropertyDescriptor[Iterable[object], Iterator[bytes]]() + def __init__(self, streaming_content: Iterable[object] = ..., *args: Any, **kwargs: Any) -> None: ... + def __iter__(self) -> Iterator[bytes]: ... def getvalue(self) -> bytes: ... class FileResponse(StreamingHttpResponse): diff --git a/tests/typecheck/views/test_function_based_views.yml b/tests/typecheck/views/test_function_based_views.yml new file mode 100644 index 0000000..21ec800 --- /dev/null +++ b/tests/typecheck/views/test_function_based_views.yml @@ -0,0 +1,102 @@ +- case: http_response + main: | + from django.http.request import HttpRequest + from django.http.response import HttpResponse + from django.utils.translation import gettext_lazy as _ + + def empty_response(request: HttpRequest) -> HttpResponse: + return HttpResponse() + + def str_response(request: HttpRequest) -> HttpResponse: + return HttpResponse('It works!') + + def bytes_response(request: HttpRequest) -> HttpResponse: + return HttpResponse(b'It works!') + + def object_response(request: HttpRequest) -> HttpResponse: + return HttpResponse(_('It works!')) + +- case: http_response_content + main: | + from django.http.request import HttpRequest + from django.http.response import HttpResponse + from django.utils.translation import gettext_lazy as _ + + def empty_response(request: HttpRequest) -> HttpResponse: + response = HttpResponse() + reveal_type(response.content) # N: Revealed type is "builtins.bytes*" + return response + + def str_response(request: HttpRequest) -> HttpResponse: + response = HttpResponse() + response.content = 'It works!' + reveal_type(response.content) # N: Revealed type is "builtins.bytes*" + return response + + def bytes_response(request: HttpRequest) -> HttpResponse: + response = HttpResponse() + response.content = b'It works!' + reveal_type(response.content) # N: Revealed type is "builtins.bytes*" + return response + + def object_response(request: HttpRequest) -> HttpResponse: + response = HttpResponse() + response.content = _('It works!') + reveal_type(response.content) # N: Revealed type is "builtins.bytes*" + return response + +- case: streaming_http_response + main: | + from django.http.request import HttpRequest + from django.http.response import StreamingHttpResponse + from django.utils.translation import gettext_lazy as _ + + def empty_response(request: HttpRequest) -> StreamingHttpResponse: + return StreamingHttpResponse() + + def str_response(request: HttpRequest) -> StreamingHttpResponse: + return StreamingHttpResponse(['It works!']) + + def bytes_response(request: HttpRequest) -> StreamingHttpResponse: + return StreamingHttpResponse([b'It works!']) + + def object_response(request: HttpRequest) -> StreamingHttpResponse: + return StreamingHttpResponse([_('It works!')]) + + def mixed_response(request: HttpRequest) -> StreamingHttpResponse: + return StreamingHttpResponse([_('Yes'), '/', _('No')]) + +- case: streaming_http_response_streaming_content + main: | + from django.http.request import HttpRequest + from django.http.response import StreamingHttpResponse + from django.utils.translation import gettext_lazy as _ + + def empty_response(request: HttpRequest) -> StreamingHttpResponse: + response = StreamingHttpResponse() + reveal_type(response.streaming_content) # N: Revealed type is "typing.Iterator*[builtins.bytes]" + return response + + def str_response(request: HttpRequest) -> StreamingHttpResponse: + response = StreamingHttpResponse() + response.streaming_content = ['It works!'] + reveal_type(response.streaming_content) # N: Revealed type is "typing.Iterator*[builtins.bytes]" + return response + + def bytes_response(request: HttpRequest) -> StreamingHttpResponse: + response = StreamingHttpResponse() + response.streaming_content = [b'It works!'] + reveal_type(response.streaming_content) # N: Revealed type is "typing.Iterator*[builtins.bytes]" + return response + + def object_response(request: HttpRequest) -> StreamingHttpResponse: + response = StreamingHttpResponse() + response.streaming_content = [_('It works!')] + reveal_type(response.streaming_content) # N: Revealed type is "typing.Iterator*[builtins.bytes]" + return response + + def mixed_response(request: HttpRequest) -> StreamingHttpResponse: + response = StreamingHttpResponse() + response.streaming_content = [_('Yes'), '/', _('No')] + reveal_type(response.streaming_content) # N: Revealed type is "typing.Iterator*[builtins.bytes]" + return response