diff --git a/zerver/lib/request.pyi b/zerver/lib/request.pyi index a31d1f9394..835d9d2214 100644 --- a/zerver/lib/request.pyi +++ b/zerver/lib/request.pyi @@ -1,14 +1,31 @@ # This mypy stubs file ensures that mypy can correctly analyze REQ. -from typing import Any, Callable, Text, TypeVar +# +# Note that here REQ is claimed to be a function, with a return type to match +# that of the parameter of which it is the default value, allowing type +# checking. However, in request.py, REQ is a class to enable the decorator to +# scan the parameter list for REQ objects and patch the parameters as the true +# types. + +from typing import Any, Callable, Text, TypeVar, Optional, Union from django.http import HttpResponse from zerver.lib.exceptions import JsonableError as JsonableError +Validator = Callable[[str, Any], Optional[str]] + +ResultT = TypeVar('ResultT') ViewFuncT = TypeVar('ViewFuncT', bound=Callable[..., HttpResponse]) class RequestVariableMissingError(JsonableError): ... class RequestVariableConversionError(JsonableError): ... -def REQ(*args: Any, **kwargs: Any) -> Any: ... +class _NotSpecified: ... +NotSpecified = _NotSpecified() + +def REQ(whence: Optional[str] = None, + converter: Optional[Callable[[str], ResultT]] = None, + default: Union[_NotSpecified, ResultT] = NotSpecified, + validator: Optional[Validator] = None, + argument_type: Optional[str] = None) -> ResultT: ... def has_request_variables(view_func: ViewFuncT) -> ViewFuncT: ... diff --git a/zerver/tests/test_decorators.py b/zerver/tests/test_decorators.py index 36d54a3654..c43f96698f 100644 --- a/zerver/tests/test_decorators.py +++ b/zerver/tests/test_decorators.py @@ -102,13 +102,13 @@ class DecoratorTestCase(TestCase): # type: () -> None def my_converter(data): - # type: (str) -> List[str] + # type: (str) -> List[int] lst = ujson.loads(data) if not isinstance(lst, list): raise ValueError('not a list') if 13 in lst: raise JsonableError('13 is an unlucky number!') - return lst + return [int(elem) for elem in lst] @has_request_variables def get_total(request, numbers=REQ(converter=my_converter)): @@ -148,7 +148,7 @@ class DecoratorTestCase(TestCase): with self.assertRaisesRegex(AssertionError, "converter and validator are mutually exclusive"): @has_request_variables def get_total(request, numbers=REQ(validator=check_list(check_int), - converter=lambda: None)): + converter=lambda x: [])): # type: (HttpRequest, Iterable[int]) -> int return sum(numbers) # nocoverage -- isn't intended to be run diff --git a/zerver/views/report.py b/zerver/views/report.py index e38b53cfb3..082394ab45 100644 --- a/zerver/views/report.py +++ b/zerver/views/report.py @@ -1,6 +1,6 @@ # System documented in https://zulip.readthedocs.io/en/latest/logging.html -from typing import Any, Dict, Optional, Text +from typing import Any, Dict, Optional, Text, Union from django.conf import settings from django.http import HttpRequest, HttpResponse @@ -33,20 +33,27 @@ def get_js_source_map() -> Optional[SourceMap]: @human_users_only @has_request_variables -def report_send_times(request, user_profile, - time=REQ(converter=to_non_negative_int), - received=REQ(converter=to_non_negative_int, default="(unknown)"), - displayed=REQ(converter=to_non_negative_int, default="(unknown)"), - locally_echoed=REQ(validator=check_bool, default=False), - rendered_content_disparity=REQ(validator=check_bool, default=False)): - # type: (HttpRequest, UserProfile, int, int, int, bool, bool) -> HttpResponse +def report_send_times(request: HttpRequest, user_profile: UserProfile, + time: int=REQ(converter=to_non_negative_int), + received: int=REQ(converter=to_non_negative_int, default=-1), + displayed: int=REQ(converter=to_non_negative_int, default=-1), + locally_echoed: bool=REQ(validator=check_bool, default=False), + rendered_content_disparity: bool=REQ(validator=check_bool, default=False)) -> HttpResponse: + received_str = "(unknown)" + if received > 0: + received_str = str(received) + displayed_str = "(unknown)" + if displayed > 0: + displayed_str = str(displayed) + request._log_data["extra"] = "[%sms/%sms/%sms/echo:%s/diff:%s]" \ - % (time, received, displayed, locally_echoed, rendered_content_disparity) + % (time, received_str, displayed_str, locally_echoed, rendered_content_disparity) + base_key = statsd_key(user_profile.realm.string_id, clean_periods=True) statsd.timing("endtoend.send_time.%s" % (base_key,), time) - if received != "(unknown)": + if received > 0: statsd.timing("endtoend.receive_time.%s" % (base_key,), received) - if displayed != "(unknown)": + if displayed > 0: statsd.timing("endtoend.displayed_time.%s" % (base_key,), displayed) if locally_echoed: statsd.incr('locally_echoed') diff --git a/zerver/webhooks/travis/view.py b/zerver/webhooks/travis/view.py index 050e5742b0..8f27d9370c 100644 --- a/zerver/webhooks/travis/view.py +++ b/zerver/webhooks/travis/view.py @@ -32,7 +32,7 @@ def api_travis_webhook(request, user_profile, ('status_message', check_string), ('compare_url', check_string), ]))): - # type: (HttpRequest, UserProfile, str, str, str, Dict[str, str]) -> HttpResponse + # type: (HttpRequest, UserProfile, str, str, bool, Dict[str, str]) -> HttpResponse message_status = message['status_message'] if ignore_pull_requests and message['type'] == 'pull_request':