request: Move miscellaneous attributes to ZulipRequestNotes.

This includes the migration of fields that require trivial changes
to be migrated to be stored with ZulipRequestNotes.

Specifically _requestor_for_logs, _set_language, _query, error_format,
placeholder_open_graph_description, saveed_response, which were all
previously set on the HttpRequest object at some point. This migration
allows them to be typed.

zerver/decorator.py

@@ -84,10 +84,11 @@ def update_user_activity(
     if request.META["PATH_INFO"] == "/json/users/me/presence":
         return
 
+    request_notes = get_request_notes(request)
     if query is not None:
         pass
-    elif hasattr(request, "_query"):
+    elif request_notes.query is not None:
-        query = request._query
+        query = request_notes.query
     else:
         query = request.META["PATH_INFO"]
 
@@ -112,7 +113,7 @@ def require_post(func: ViewFuncT) -> ViewFuncT:
                 request.path,
                 extra={"status_code": 405, "request": request},
             )
-            if request.error_format == "JSON":
+            if get_request_notes(request).error_format == "JSON":
                 return json_method_not_allowed(["POST"])
             else:
                 return TemplateResponse(
@@ -442,7 +443,7 @@ def do_login(request: HttpRequest, user_profile: UserProfile) -> None:
 def log_view_func(view_func: ViewFuncT) -> ViewFuncT:
     @wraps(view_func)
     def _wrapped_view_func(request: HttpRequest, *args: object, **kwargs: object) -> HttpResponse:
-        request._query = view_func.__name__
+        get_request_notes(request).query = view_func.__name__
         return view_func(request, *args, **kwargs)
 
     return cast(ViewFuncT, _wrapped_view_func)  # https://github.com/python/mypy/issues/1927

zerver/lib/i18n.py

@@ -10,6 +10,8 @@ from django.conf import settings
 from django.http import HttpRequest
 from django.utils import translation
 
+from zerver.lib.request import get_request_notes
+
 
 @lru_cache()
 def get_language_list() -> List[Dict[str, Any]]:
@@ -62,6 +64,6 @@ def get_and_set_request_language(
     # something reasonable will happen in logged-in portico pages.
     # We accomplish that by setting a flag on the request which signals
     # to LocaleMiddleware to set the cookie on the response.
-    request._set_language = translation.get_language()
+    get_request_notes(request).set_language = translation.get_language()
 
     return request_language

zerver/lib/rest.py

@@ -14,6 +14,7 @@ from zerver.decorator import (
     process_as_post,
 )
 from zerver.lib.exceptions import MissingAuthenticationError
+from zerver.lib.request import get_request_notes
 from zerver.lib.response import json_method_not_allowed
 from zerver.lib.types import ViewFuncT
 
@@ -66,11 +67,11 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
     etc, as that is where we route HTTP verbs to target functions.
     """
     supported_methods: Dict[str, Any] = {}
-
+    request_notes = get_request_notes(request)
-    if hasattr(request, "saved_response"):
+    if request_notes.saved_response is not None:
         # For completing long-polled Tornado requests, we skip the
         # view function logic and just return the response.
-        return request.saved_response
+        return request_notes.saved_response
 
     # duplicate kwargs so we can mutate the original as we go
     for arg in list(kwargs):
@@ -99,9 +100,9 @@ def rest_dispatch(request: HttpRequest, **kwargs: Any) -> HttpResponse:
             target_function = supported_methods[method_to_use]
             view_flags = set()
 
-        # Set request._query for update_activity_user(), which is called
+        # Set request_notes.query for update_activity_user(), which is called
         # by some of the later wrappers.
-        request._query = target_function.__name__
+        request_notes.query = target_function.__name__
 
         # We want to support authentication by both cookies (web client)
         # and API keys (API clients). In the former case, we want to

zerver/middleware.py

@@ -346,7 +346,7 @@ class LogRequests(MiddlewareMixin):
         if request_notes.log_data is not None:
             # Sanity check to ensure this is being called from the
             # Tornado code path that returns responses asynchronously.
-            assert getattr(request, "saved_response", False)
+            assert request_notes.saved_response is not None
 
             # Avoid re-initializing request_notes.log_data if it's already there.
             return
@@ -363,7 +363,7 @@ class LogRequests(MiddlewareMixin):
         kwargs: Dict[str, Any],
     ) -> None:
         request_notes = get_request_notes(request)
-        if hasattr(request, "saved_response"):
+        if request_notes.saved_response is not None:
             # The below logging adjustments are unnecessary (because
             # we've already imported everything) and incorrect
             # (because they'll overwrite data from pre-long-poll
@@ -458,7 +458,7 @@ class JsonErrorHandler(MiddlewareMixin):
 
         if isinstance(exception, JsonableError):
             return json_response_from_error(exception)
-        if request.error_format == "JSON":
+        if get_request_notes(request).error_format == "JSON":
             capture_exception(exception)
             json_error_logger = logging.getLogger("zerver.middleware.json_error_handler")
             json_error_logger.error(traceback.format_exc(), extra=dict(request=request))
@@ -474,9 +474,9 @@ class TagRequests(MiddlewareMixin):
 
     def process_request(self, request: HttpRequest) -> None:
         if request.path.startswith("/api/") or request.path.startswith("/json/"):
-            request.error_format = "JSON"
+            get_request_notes(request).error_format = "JSON"
         else:
-            request.error_format = "HTML"
+            get_request_notes(request).error_format = "HTML"
 
 
 class CsrfFailureError(JsonableError):
@@ -493,7 +493,7 @@ class CsrfFailureError(JsonableError):
 
 
 def csrf_failure(request: HttpRequest, reason: str = "") -> HttpResponse:
-    if request.error_format == "JSON":
+    if get_request_notes(request).error_format == "JSON":
         return json_response_from_error(CsrfFailureError(reason))
     else:
         return html_csrf_failure(request, reason)
@@ -516,9 +516,10 @@ class LocaleMiddleware(DjangoLocaleMiddleware):
 
         # An additional responsibility of our override of this middleware is to save the user's language
         # preference in a cookie. That determination is made by code handling the request
-        # and saved in the _set_language flag so that it can be used here.
+        # and saved in the set_language flag so that it can be used here.
-        if hasattr(request, "_set_language"):
+        set_language = get_request_notes(request).set_language
-            response.set_cookie(settings.LANGUAGE_COOKIE_NAME, request._set_language)
+        if set_language is not None:
+            response.set_cookie(settings.LANGUAGE_COOKIE_NAME, set_language)
 
         return response
 
@@ -611,8 +612,12 @@ class SetRemoteAddrFromRealIpHeader(MiddlewareMixin):
 
 def alter_content(request: HttpRequest, content: bytes) -> bytes:
     first_paragraph_text = get_content_description(content, request)
+    placeholder_open_graph_description = get_request_notes(
+        request
+    ).placeholder_open_graph_description
+    assert placeholder_open_graph_description is not None
     return content.replace(
-        request.placeholder_open_graph_description.encode("utf-8"),
+        placeholder_open_graph_description.encode("utf-8"),
         first_paragraph_text.encode("utf-8"),
     )
 
@@ -622,7 +627,7 @@ class FinalizeOpenGraphDescription(MiddlewareMixin):
         self, request: HttpRequest, response: StreamingHttpResponse
     ) -> StreamingHttpResponse:
 
-        if getattr(request, "placeholder_open_graph_description", None) is not None:
+        if get_request_notes(request).placeholder_open_graph_description is not None:
             assert not response.streaming
             response.content = alter_content(request, response.content)
         return response

zerver/tornado/handlers.py

@@ -250,7 +250,7 @@ class AsyncDjangoHandler(tornado.web.RequestHandler, base.BaseHandler):
         # request/middleware system to run unmodified while avoiding
         # running expensive things like Zulip's authentication code a
         # second time.
-        request.saved_response = json_response(
+        request_notes.saved_response = json_response(
             res_type=result_dict["result"], data=result_dict, status=self.get_status()
         )
 

zerver/views/documentation.py

@@ -13,7 +13,7 @@ from django.views.generic import TemplateView
 from zerver.context_processors import zulip_default_context
 from zerver.decorator import add_google_analytics_context
 from zerver.lib.integrations import CATEGORIES, INTEGRATIONS, HubotIntegration, WebhookIntegration
-from zerver.lib.request import REQ, has_request_variables
+from zerver.lib.request import REQ, get_request_notes, has_request_variables
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.templates import render_markdown_path
 from zerver.models import Realm
@@ -168,10 +168,11 @@ class MarkdownDirectoryView(ApiURLView):
                 context["OPEN_GRAPH_TITLE"] = f"{article_title} ({title_base})"
             else:
                 context["OPEN_GRAPH_TITLE"] = title_base
-            self.request.placeholder_open_graph_description = (
+            request_notes = get_request_notes(self.request)
+            request_notes.placeholder_open_graph_description = (
                 f"REPLACMENT_OPEN_GRAPH_DESCRIPTION_{int(2**24 * random.random())}"
             )
-            context["OPEN_GRAPH_DESCRIPTION"] = self.request.placeholder_open_graph_description
+            context["OPEN_GRAPH_DESCRIPTION"] = request_notes.placeholder_open_graph_description
 
         context["sidebar_index"] = sidebar_index
         # An "article" might require the api_uri_context to be rendered