events: Fix documentation testing for /events.

Most of the failures were due to parameters that are not intended to
be used by third-party code, so the correct fix for those was the set
intentionally_undocumented=True.

Fixes #12969.

zerver/lib/request.py

@@ -188,12 +188,6 @@ def has_request_variables(view_func: ViewFuncT) -> ViewFuncT:
     post_params = []
 
     view_func_full_name = '.'.join([view_func.__module__, view_func.__name__])
-    if view_func_full_name == 'zerver.tornado.views.get_events_backend':
-        # Hack to help test_openapi find the has_request_variables
-        # data for get_events.  This is complex because of how
-        # get_events_backend, which parses the parameters, is wrapped
-        # by multiple callers that don't parse the parameters.
-        view_func_full_name = 'zerver.tornado.views.get_events'
 
     for (name, value) in zip(default_param_names, default_param_values):
         if isinstance(value, _REQ):

zerver/tests/test_openapi.py

@@ -232,11 +232,11 @@ class OpenAPIArgumentsTest(ZulipTestCase):
         '/user_groups/{group_id}',  # Equivalent of what's in urls.py
         '/user_groups/{user_group_id}',  # What's in the OpenAPI docs
     ])
-    # TODO: These endpoints have a mismatch between the
-    # documentation and the actual API and need to be fixed:
+
+    # Endpoints where the documentation is currently failing our
+    # consistency tests.  We aim to keep this list empty.
     buggy_documentation_endpoints = set([
-        '/events',
-    ])
+    ])  # type: Set[str]
 
     def convert_regex_to_url_pattern(self, regex_pattern: str) -> str:
         """ Convert regular expressions style URL patterns to their
@@ -481,6 +481,16 @@ do not match the types declared in the implementation of {}.\n""".format(functio
                 else:
                     function_name, tags = value
 
+                if function_name == 'zerver.tornado.views.get_events':
+                    # Work around the fact that the registered
+                    # get_events view function isn't where we do
+                    # @has_request_variables.
+                    #
+                    # TODO: Make this configurable via an optional argument
+                    # to has_request_variables, e.g.
+                    # @has_request_variables(view_func_name="zerver.tornado.views.get_events")
+                    function_name = 'zerver.tornado.views.get_events_backend'
+
                 lookup_parts = function_name.split('.')
                 module = __import__('.'.join(lookup_parts[:-1]), {}, {}, [''])
                 function = getattr(module, lookup_parts[-1])
@@ -540,7 +550,7 @@ so maybe we shouldn't include it in pending_endpoints.
                     [parameter['name'] for parameter in openapi_parameters]
                 )
 
-                if len(accepted_arguments - openapi_parameter_names) > 0:
+                if len(accepted_arguments - openapi_parameter_names) > 0:  # nocoverage
                     print("Undocumented parameters for",
                           url_pattern, method, function_name)
                     print(" +", openapi_parameter_names)

zerver/tornado/views.py

@@ -1,5 +1,5 @@
 import time
-from typing import Iterable, List, Optional, Sequence, Union
+from typing import Iterable, Optional, Sequence, Union
 
 import ujson
 from django.core.handlers.base import BaseHandler
@@ -50,16 +50,29 @@ def get_events(request: HttpRequest, user_profile: UserProfile,
 
 @has_request_variables
 def get_events_backend(request: HttpRequest, user_profile: UserProfile, handler: BaseHandler,
-                       user_client: Optional[Client]=REQ(converter=get_client, default=None),
+                       # user_client is intended only for internal Django=>Tornado requests
+                       # and thus shouldn't be documented for external use.
+                       user_client: Optional[Client]=REQ(converter=get_client, default=None,
+                                                         intentionally_undocumented=True),
                        last_event_id: Optional[int]=REQ(converter=int, default=None),
-                       queue_id: Optional[List[str]]=REQ(default=None),
-                       apply_markdown: bool=REQ(default=False, validator=check_bool),
-                       client_gravatar: bool=REQ(default=False, validator=check_bool),
-                       all_public_streams: bool=REQ(default=False, validator=check_bool),
-                       event_types: Optional[str]=REQ(default=None, validator=check_list(check_string)),
+                       queue_id: Optional[str]=REQ(default=None),
+                       # apply_markdown, client_gravatar, all_public_streams, and various
+                       # other parameters are only used when registering a new queue via this
+                       # endpoint.  This is a feature used primarily by get_events_internal
+                       # and not expected to be used by third-party clients.
+                       apply_markdown: bool=REQ(default=False, validator=check_bool,
+                                                intentionally_undocumented=True),
+                       client_gravatar: bool=REQ(default=False, validator=check_bool,
+                                                 intentionally_undocumented=True),
+                       all_public_streams: bool=REQ(default=False, validator=check_bool,
+                                                    intentionally_undocumented=True),
+                       event_types: Optional[str]=REQ(default=None, validator=check_list(check_string),
+                                                      intentionally_undocumented=True),
                        dont_block: bool=REQ(default=False, validator=check_bool),
-                       narrow: Iterable[Sequence[str]]=REQ(default=[], validator=check_list(None)),
-                       lifespan_secs: int=REQ(default=0, converter=to_non_negative_int)
+                       narrow: Iterable[Sequence[str]]=REQ(default=[], validator=check_list(None),
+                                                           intentionally_undocumented=True),
+                       lifespan_secs: int=REQ(default=0, converter=to_non_negative_int,
+                                              intentionally_undocumented=True)
                        ) -> Union[HttpResponse, _RespondAsynchronously]:
     if user_client is None:
         valid_user_client = request.client