From 7e6bcbeac01edcacbb9f136ec056026f89e8fcf3 Mon Sep 17 00:00:00 2001 From: "Hemanth V. Alluri" Date: Mon, 1 Jul 2019 16:52:54 +0530 Subject: [PATCH] openapi: Allow us to specify an endpoint as being undocumented in urls. In each url of urls.py, if we want to mark an endpoint as being intentionally undocumented, then in the kwargs instead of directly mapping like 'METHOD': 'zerver.views.package.foo', we can provide a tag called 'intentionally_undocumented' and map like: 'METHOD': ('zerver.views.package.foo', {'intentionally_undocumented'}). If an endpoint is marked as intentionally undocumented, but we find some OpenAPI documentation for it then we'll throw an error, in which case either the 'intentionally_undocumented' tag should be removed or the faulty documentation should be removed. --- zerver/tests/test_openapi.py | 36 +++++++++++++++++------------------- zproject/urls.py | 27 ++++++++++++++++++++------- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index c9d48f5397..beebbca517 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import mock -from typing import Dict, Any +from typing import Dict, Any, Set from django.conf import settings @@ -147,21 +147,8 @@ class OpenAPIArgumentsTest(ZulipTestCase): # Verifies that every REQ-defined argument appears in our API # documentation for the target endpoint where possible. - # TODO: Potentially this should move into configuration flags - # we attach to the endpoint in zproject/urls.py, to indicate - # it's an endpoint with no documentation by design. - NO_API_DOCS = set([ - # These endpoints are for webapp-only use and will likely - # never have docs. - '/report/narrow_times', - '/report/send_times', - '/report/unnarrow_times', - '/report/error', - '/users/me/tutorial_status', - '/users/me/hotspots', - # These should probably be eliminated - '/users/me/enter-sends', - # These should have docs added + # These should have docs added + PENDING_ENDPOINTS = set([ '/users/me/avatar', '/user_uploads', '/settings/display', @@ -232,8 +219,9 @@ class OpenAPIArgumentsTest(ZulipTestCase): for method, value in p.default_args.items(): if isinstance(value, str): function = value + tags = set() # type: Set[str] else: - function = value[0] + function, tags = value # Our accounting logic in the `has_request_variables()` # code means we have the list of all arguments # accepted by every view function in arguments_map. @@ -258,10 +246,20 @@ class OpenAPIArgumentsTest(ZulipTestCase): # some URLs. url_pattern = '/' + regex_pattern[1:][:-1] - if url_pattern in NO_API_DOCS: + if url_pattern in PENDING_ENDPOINTS: + # TODO: Once these endpoints have been aptly documented, + # we should remove this block and the associated List. + continue + if "intentionally_undocumented" in tags: # Don't do any validation on endpoints with no API # documentation by design. - continue + try: + get_openapi_parameters(url_pattern, method) + raise AssertionError("We found some OpenAPI \ +documentation for %s %s, so maybe we shouldn't mark it as intentionally \ +undocumented in the urls." % (method, url_pattern)) + except KeyError: + continue if "(?P<" in url_pattern: # See above TODO about our matching algorithm not # handling captures in the regular expressions. diff --git a/zproject/urls.py b/zproject/urls.py index 936fcca62f..c191fd2c2a 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -284,18 +284,26 @@ v1_api_and_json_patterns = [ url(r'^users/me/api_key/regenerate$', rest_dispatch, {'POST': 'zerver.views.user_settings.regenerate_api_key'}), url(r'^users/me/enter-sends$', rest_dispatch, - {'POST': 'zerver.views.user_settings.change_enter_sends'}), + {'POST': ('zerver.views.user_settings.change_enter_sends', + # This endpoint should be folded into user settings + {'intentionally_undocumented'})}), url(r'^users/me/avatar$', rest_dispatch, {'POST': 'zerver.views.user_settings.set_avatar_backend', 'DELETE': 'zerver.views.user_settings.delete_avatar_backend'}), # users/me/hotspots -> zerver.views.hotspots url(r'^users/me/hotspots$', rest_dispatch, - {'POST': 'zerver.views.hotspots.mark_hotspot_as_read'}), + {'POST': ('zerver.views.hotspots.mark_hotspot_as_read', + # This endpoint is low priority for documentation as + # it is part of the webapp-specific tutorial. + {'intentionally_undocumented'})}), # users/me/tutorial_status -> zerver.views.tutorial url(r'^users/me/tutorial_status$', rest_dispatch, - {'POST': 'zerver.views.tutorial.set_tutorial_status'}), + {'POST': ('zerver.views.tutorial.set_tutorial_status', + # This is a relic of an old Zulip tutorial model and + # should be deleted. + {'intentionally_undocumented'})}), # settings -> zerver.views.user_settings url(r'^settings$', rest_dispatch, @@ -370,15 +378,20 @@ v1_api_and_json_patterns = [ 'DELETE': 'zerver.tornado.views.cleanup_event_queue'}), # report -> zerver.views.report + # + # These endpoints are for internal error/performance reporting + # from the browser to the webapp, and we don't expect to ever + # include in our API documentation. url(r'^report/error$', rest_dispatch, # Logged-out browsers can hit this endpoint, for portico page JS exceptions. - {'POST': ('zerver.views.report.report_error', {'allow_anonymous_user_web'})}), + {'POST': ('zerver.views.report.report_error', {'allow_anonymous_user_web', + 'intentionally_undocumented'})}), url(r'^report/send_times$', rest_dispatch, - {'POST': 'zerver.views.report.report_send_times'}), + {'POST': ('zerver.views.report.report_send_times', {'intentionally_undocumented'})}), url(r'^report/narrow_times$', rest_dispatch, - {'POST': 'zerver.views.report.report_narrow_times'}), + {'POST': ('zerver.views.report.report_narrow_times', {'intentionally_undocumented'})}), url(r'^report/unnarrow_times$', rest_dispatch, - {'POST': 'zerver.views.report.report_unnarrow_times'}), + {'POST': ('zerver.views.report.report_unnarrow_times', {'intentionally_undocumented'})}), # Used to generate a Zoom video call URL url(r'^calls/create$', rest_dispatch,