Browse Source

fix(docs) Improve generated API documetation (#9801)

Fix several issues in documentation generation:

* Add the request path
* The description needs to be joined on spaces to not awkwardly join multi line descriptions.
* Handle multi-line parameter descriptions better.
* Extract warnings into a separate property.
* Rename path to api_path as path is reserved in jekyll
* Fix incorrect title being used.
* Fix incorrect key usage in markdown dump
* Remove the 'Return' docs on an endpoint as it isn't useful given that
  the docs have example responses.
* Fix a few awkward pluralizations.
* Retain inline markup from RST to markdown conversion.
Mark Story 6 years ago
parent
commit
facec83a30

+ 5 - 3
api-docs/generator.py

@@ -240,13 +240,15 @@ def output_markdown(sections, scenarios, section_mapping):
             if len(endpoint['params'].get('auth', [])):
                 auth = endpoint['params']['auth'][0]['description']
             payload = dict(
-                title=title,
+                title=endpoint['title'],
                 sidebar_order=i,
-                description=''.join(endpoint['text']),
+                description='\n'.join(endpoint['text']).strip(),
+                warning=endpoint['warning'],
                 method=endpoint['method'],
+                api_path=endpoint['path'],
                 query_parameters=endpoint['params'].get('query'),
                 path_parameters=endpoint['params'].get('path'),
-                parameters=endpoint['params'].get('params'),
+                parameters=endpoint['params'].get('param'),
                 authentication=auth,
                 example_request=format_request(endpoint, scenarios),
                 example_response=format_response(endpoint, scenarios)

+ 2 - 2
src/sentry/api/endpoints/dsym_files.py

@@ -132,8 +132,8 @@ class DSymFilesEndpoint(ProjectEndpoint):
 
     def post(self, request, project):
         """
-        Upload a New Files
-        ``````````````````
+        Upload a New File
+        `````````````````
 
         Upload a new dsym file for the given release.
 

+ 0 - 7
src/sentry/api/endpoints/organization_eventid.py

@@ -36,14 +36,7 @@ class EventIdLookupEndpoint(OrganizationEndpoint):
                                           event ID should be looked up in.
         :param string event_id: the event ID to look up.
         :auth: required
-
-        Return:
-            organizationSlug
-            projectSlug
-            groupId
-            eventId (optional)
         """
-
         # Largely copied from ProjectGroupIndexEndpoint
         if len(event_id) != 32:
             return Response({'detail': 'Event ID must be 32 characters.'}, status=400)

+ 2 - 2
src/sentry/api/endpoints/project_servicehook_details.py

@@ -17,8 +17,8 @@ class ProjectServiceHookDetailsEndpoint(ProjectEndpoint):
 
     def get(self, request, project, hook_id):
         """
-        Retrieve a Service Hooks
-        ````````````````````````
+        Retrieve a Service Hook
+        ```````````````````````
 
         Return a service hook bound to a project.
 

+ 59 - 11
src/sentry/utils/apidocs.py

@@ -26,6 +26,8 @@ non_named_group_matcher = re.compile(r'\([^\)]+\)')
 # [foo|bar|baz]
 either_option_matcher = re.compile(r'\[([^\]]+)\|([^\]]+)\]')
 camel_re = re.compile(r'([A-Z]+)([a-z])')
+rst_indent_re = re.compile(r'^\s{2,}')
+rst_block_re = re.compile(r'^\.\.\s[a-z]+::$')
 
 API_PREFIX = '/api/0/'
 
@@ -82,42 +84,86 @@ def get_endpoint_path(internal_endpoint):
 
 
 def parse_doc_string(doc):
+    """
+    Parse a docstring into a tuple.
+
+    The tuple contains:
+
+    (title, lines, warning, params)
+
+    `lines` is a list for backwards compatibility with
+    the JSON formatter.
+    """
     title = None
     current_param = ''
+    in_warning = False
     param_lines = []
     lines = []
+    warning = []
     iterable = iter((doc or u'').splitlines())
 
     for line in iterable:
-        line = line.strip()
+        stripped = line.strip()
         if title is None:
             if not line:
                 continue
-            title = line
-        elif line and line[0] * len(line) == line:
+            title = line.strip()
+        elif stripped and stripped[0] * len(stripped) == stripped:
             # is an RST underline
             continue
-        elif line and line.startswith(':'):
+        elif rst_block_re.match(stripped):
+            # Presently the only RST block we use is `caution` which
+            # displays as a 'warning'
+            in_warning = True
+        elif line and stripped.startswith(':'):
             # Is a new parameter or other annotation
             if current_param:
                 param_lines.append(current_param)
-            current_param = line
+            current_param = stripped
         elif current_param:
             # Adding to an existing parameter annotation
-            current_param = current_param + ' ' + line.strip()
-        elif line:
-            lines.append(line)
+            current_param = current_param + ' ' + stripped
+        else:
+            if in_warning:
+                # If we're indented at least 2 spaces assume
+                # we're in the RST block
+                if rst_indent_re.match(line) or not line:
+                    warning.append(stripped)
+                    continue
+                # An un-indented non-empty line means we
+                # have other content.
+                elif line:
+                    in_warning = False
+            # Normal text. We want empty lines here so we can
+            # preserve paragraph breaks.
+            lines.append(stripped)
 
     if current_param:
         param_lines.append(current_param)
 
-    return title, lines, parse_params(param_lines)
+    if warning:
+        warning = '\n'.join(warning).strip()
+    if not warning:
+        warning = None
+
+    return title, lines, warning, parse_params(param_lines)
 
 
 def get_node_text(nodes):
     """Recursively read text from a node tree."""
     text = []
+    format_tags = {
+        'literal': '`',
+        'strong': '**',
+        'emphasis': '*',
+    }
     for node in nodes:
+        # Handle inline formatting elements.
+        if (node.nodeType == node.ELEMENT_NODE and
+                node.tagName in format_tags):
+            wrap = format_tags[node.tagName]
+            text.append(wrap + get_node_text(node.childNodes) + wrap)
+            continue
         if node.nodeType == node.TEXT_NODE:
             text.append(node.data)
         if node.nodeType == node.ELEMENT_NODE:
@@ -198,12 +244,13 @@ def extract_endpoint_info(pattern, internal_endpoint):
         if endpoint_name.endswith('Endpoint'):
             endpoint_name = endpoint_name[:-8]
         endpoint_name = camelcase_to_dashes(endpoint_name)
-        title, text, params = parse_doc_string(doc)
+        title, text, warning, params = parse_doc_string(doc)
         yield dict(
             path=API_PREFIX + path.lstrip('/'),
             method=method_name,
             title=title,
             text=text,
+            warning=warning,
             params=params,
             scenarios=getattr(method, 'api_scenarios', None) or [],
             section=section.name.lower(),
@@ -556,11 +603,12 @@ class Runner(object):
         """Convert the current scenario into a dict
         """
         doc = extract_documentation(self.func)
-        title, text, params = parse_doc_string(doc)
+        title, text, warning, params = parse_doc_string(doc)
         return {
             'ident': self.ident,
             'requests': self.requests,
             'title': title,
             'text': text,
             'params': params,
+            'warning': warning,
         }

+ 57 - 9
tests/sentry/utils/test_apidocs.py

@@ -15,29 +15,77 @@ def test_parse_doc_string():
 Glory!
 ~~~~~~
 
-This is a glorious function
+This is a glorious function.
 
-:param int id: An id.
-:qparam bool louder: Make it louder?
-:pparam string orgid: The orgid
+It has a multi-line description.
+
+:param int id: An ``id``.
+:qparam bool louder: Make it **louder**?
+:pparam string orgid: The *orgid*.
+                      On a second line.
 :auth: required
 """
     result = apidocs.parse_doc_string(text)
     assert result[0] == 'Glory!'
-    assert result[1] == ['This is a glorious function']
-    params = result[2]
+    expected = [
+        '',
+        'This is a glorious function.',
+        '',
+        'It has a multi-line description.',
+        ''
+    ]
+    assert result[1] == expected
+    assert result[2] is None, 'no warning present'
+
+    params = result[3]
     assert 'path' in params, 'Should have path params'
     assert params['path'][0] == dict(name='orgid', type="string",
-                                     description='The orgid')
+                                     description='The *orgid*. On a second line.')
     assert 'query' in params, 'Should have query params'
     assert params['query'][0] == dict(name='louder', type="bool",
-                                     description='Make it louder?')
+                                      description='Make it **louder**?')
     assert 'auth' in params, 'Should have auth'
     assert params['auth'][0] == dict(name='', type="", description='required')
 
     assert 'param' in params, 'Should have regular param'
     assert params['param'][0] == dict(name='id', type="int",
-                                     description='An id.')
+                                      description='An `id`.')
+
+
+def test_parse_doc_string_warning():
+    text = """
+Danger!
+~~~~~~~
+
+This is a dangerous function.
+
+.. warning::
+    This is a warning
+
+    It has multiple lines
+
+It has a multi-line description.
+
+:param int id: An id.
+"""
+    result = apidocs.parse_doc_string(text)
+    assert result[0] == 'Danger!'
+    expected = [
+        '',
+        'This is a dangerous function.',
+        '',
+        'It has a multi-line description.',
+        ''
+    ]
+    assert result[1] == expected
+
+    expected = 'This is a warning\n\nIt has multiple lines'
+    assert result[2] == expected
+
+    params = result[3]
+    assert 'param' in params, 'Should have regular param'
+    assert params['param'][0] == dict(name='id', type="int",
+                                      description='An id.')
 
 
 def test_camelcase_to_dashes():