Mutiple changes to REST code.

Requesting an invalid attribut via rest/data/class/id/attrib used to
return a 405, it now returns a 400 and a better error message.

/rest/ response scans the registered endpoints rather than using a
hard coded description. So new endpoints added in interfaces.py are
listed.

Fix a number of Allow headers that were listing invalid methods. Also
when invalid method is used, report valid methods in response. Extract
methods from Route list.

Fix Access-Control-Allow-Methods. Add X-Requested-With to
Access-Control-Allow-Headers.

Add decorator openapi_doc to add openapi annotations for the rest
endpoints.  Added a couple of examples. Returning this info to a
client is still a work in progress.

Author: rouilj

CHANGES.txt

@@ -38,6 +38,9 @@ Fixed:
 - issue2551167 - pip install in containerized environments puts
   template and locale files under site-packages where roundup can't find
   them. Change code to find them under site-packages.
+- REST replace hard coded list of child endpoints for /rest/ with list
+  pulled from registered endpoints. So newly added endpoints are
+  shown. (John Rouillard)
 
 Features:
 
@@ -61,6 +64,15 @@ Features:
   "-V HTTP/1.0" can turn it off. (John Rouillard)
 - issue2551163 - add scripts/Dockerfile to provide basic support for
   containerization. See installation.txt for details. (John Rouillard)
+- REST add openapi_doc decorator to add openapi_doc to
+  endpoints. Decorate a couple of examples. (John Rouillard)
+- REST when incorrect method is used, report allowed methods in error
+  message as well as in an Allow header. (John Rouillard)
+- REST change response to invalid attribute specified in path.  Return
+  400 code not 405 code for this case and improve error. (John
+  Rouillard)
+- REST correct values for some Access-Control-Allow-Methods and
+  Access-Control-Allow-Headers headers. (John Rouillard)
 
 2021-07-13 2.1.0
 

roundup/rest.py

@@ -123,8 +123,51 @@ def format_object(self, *args, **kwargs):
                 'data': data
             }
         return result
+
+    format_object.wrapped_func = func
     return format_object
 
+def openapi_doc(d):
+    """Annotate rest routes with openapi data. Takes a dict
+       for the openapi spec. It can be used standalone
+       as the openapi spec paths.<path>.<method> =
+
+     {
+        "summary": "this path gets a value",
+        "description": "a longer description",
+        "responses": {
+          "200": {
+            "description": "normal response",
+            "content": {
+              "application/json": {},
+              "application/xml": {}
+            }
+          },
+          "406": {
+            "description": "Unable to provide requested content type",
+            "content": {
+              "application/json": {}
+            }
+          }
+        },
+        "parameters": [
+          {
+            "$ref": "#components/parameters/generic_.stats"
+          },
+          {
+            "$ref": "#components/parameters/generic_.apiver"
+          },
+          {
+            "$ref": "#components/parameters/generic_.verbose"
+          }
+        ]
+     }
+    """
+
+    def wrapper(f):
+        f.openapi_doc = d
+        return f
+    return wrapper
 
 def calculate_etag(node, key, classname="Missing", id="0",
                    repr_format="json"):
@@ -347,7 +390,13 @@ def execute(cls, instance, path, method, input):
                 try:
                     func_obj = funcs[method]
                 except KeyError:
-                    raise Reject('Method %s not allowed' % method)
+                    valid_methods = ', '.join(sorted(funcs.keys()))
+                    raise Reject(_('Method %(m)s not allowed. '
+                                   'Allowed: %(a)s')% {
+                                       'm': method,
+                                       'a': valid_methods
+                                   },
+                                   valid_methods)
 
                 # retrieve the vars list and the function caller
                 list_vars = func_obj['vars']
@@ -891,6 +940,7 @@ def get_collection(self, class_name, input):
 
         result['@total_size'] = result_len
         self.client.setHeader("X-Count-Total", str(result_len))
+        self.client.setHeader("Allow", "OPTIONS, GET, POST")
         return 200, result
 
     @Routing.route("/data/<:class_name>/<:item_id>", 'GET')
@@ -1028,7 +1078,10 @@ def get_attribute(self, class_name, item_id, attr_name, input):
         node = class_obj.getnode(item_id)
         etag = calculate_etag(node, self.db.config.WEB_SECRET_KEY,
                               class_name, item_id,  repr_format="json")
-        data = node.__getattr__(attr_name)
+        try:
+            data = node.__getattr__(attr_name)
+        except AttributeError as e:
+            raise UsageError(_("Invalid attribute %s"%attr_name))
         result = {
             'id': item_id,
             'type': str(type(data)),
@@ -1189,6 +1242,15 @@ def post_collection_inner(self, class_name, input):
         link = '%s/%s/%s' % (self.data_path, class_name, item_id)
         self.client.setHeader("Location", link)
 
+        self.client.setHeader(
+            "Allow",
+            None
+        )
+        self.client.setHeader(
+            "Access-Control-Allow-Methods",
+            None
+        )
+
         # set the response body
         result = {
             'id': item_id,
@@ -1643,6 +1705,11 @@ def options_collection(self, class_name, input):
             "Allow",
             "OPTIONS, GET, POST"
         )
+
+        self.client.setHeader(
+            "Access-Control-Allow-Methods",
+            "OPTIONS, GET, POST"
+        )
         return 204, ""
 
     @Routing.route("/data/<:class_name>/<:item_id>", 'OPTIONS')
@@ -1698,19 +1765,114 @@ def option_attribute(self, class_name, item_id, attr_name, input):
                 attr_name, class_name))
         return 204, ""
 
+    @openapi_doc({"summary": "Describe Roundup rest endpoint.",
+                  "description": ("Report all supported api versions "
+                                  "and default api version. "
+                                  "Also report next level of link "
+                                  "endpoints below /rest endpoint"),
+                  "responses": {
+                      "200": {
+                          "description": "Successful response.",
+                          "content": {
+                              "application/json": {
+                              "examples": {
+                                  "success": {
+                                      "summary": "Normal json data.",
+                                      "value": """{
+  "data": {
+    "default_version": 1,
+    "supported_versions": [
+      1
+    ],
+    "links": [
+      {
+        "uri": "https://tracker.example.com/demo/rest",
+        "rel": "self"
+      },
+      {
+        "uri": "https://tracker.example.com/demo/rest/data",
+        "rel": "data"
+      },
+      {
+        "uri": "https://tracker.example.com/demo/rest/summary",
+        "rel": "summary"
+      }
+    ]
+  }
+}"""
+                                  }
+                              }
+                          },
+                          "application/xml": {
+                              "examples": {
+                                  "success": {
+                                      "summary": "Normal xml data",
+                                      "value": """<dataf type="dict">
+  <default_version type="int">1</default_version>
+  <supported_versions type="list">
+    <item type="int">1</item>
+  </supported_versions>
+  <links type="list">
+    <item type="dict">
+      <uri type="str">https://rouilj.dynamic-dns.net/sysadmin/rest</uri>
+      <rel type="str">self</rel>
+    </item>
+    <item type="dict">
+      <uri type="str">https://rouilj.dynamic-dns.net/sysadmin/rest/data</uri>
+      <rel type="str">data</rel>
+    </item>
+    <item type="dict">
+      <uri type="str">https://rouilj.dynamic-dns.net/sysadmin/rest/summary</uri>
+      <rel type="str">summary</rel>
+    </item>
+    <item type="dict">
+      <uri type="str">https://rouilj.dynamic-dns.net/sysadmin/rest/summary2</uri>
+      <rel type="str">summary2</rel>
+    </item>
+  </links>
+</dataf>"""
+                                  }
+                              }
+                          }
+                        }
+                      }
+                  }
+    }
+    )
     @Routing.route("/")
     @_data_decorator
     def describe(self, input):
-        """Describe the rest endpoint"""
+        """Describe the rest endpoint. Return direct children in
+           links list.
+        """
+
+        # paths looks like ['^rest/$', '^rest/summary$',
+        #                   '^rest/data/<:class>$', ...]
+        paths = Routing._Routing__route_map.keys()
+
+        links = []
+        # p[1:-1] removes ^ and $ from regexp
+        # if p has only 1 /, it's a child of rest/ root.
+        child_paths = sorted([ p[1:-1] for p in paths if
+                               p.count('/') == 1 ])
+        for p in child_paths:
+            # p.split('/')[1] is the residual path after
+            # removing rest/. child_paths look like:
+            # ['rest/', 'rest/summary'] etc.
+            rel = p.split('/')[1]
+            if rel:
+                rel_path = "/" + rel
+            else:
+                rel_path = rel
+                rel = "self"
+            links.append( {"uri": self.base_path + rel_path,
+                           "rel": rel
+                           })
+
         result = {
             "default_version": self.__default_api_version,
             "supported_versions": self.__supported_api_versions,
-            "links": [{"uri": self.base_path + "/summary",
-                       "rel": "summary"},
-                      {"uri": self.base_path,
-                       "rel": "self"},
-                      {"uri": self.base_path + "/data",
-                       "rel": "data"}]
+            "links": links
         }
 
         return 200, result
@@ -1983,15 +2145,15 @@ def dispatch(self, method, uri, input):
         self.client.setHeader("Access-Control-Allow-Origin", "*")
         self.client.setHeader(
             "Access-Control-Allow-Headers",
-            "Content-Type, Authorization, X-HTTP-Method-Override"
+            "Content-Type, Authorization, X-Requested-With, X-HTTP-Method-Override"
         )
         self.client.setHeader(
             "Allow",
             "OPTIONS, GET, POST, PUT, DELETE, PATCH"
         )
         self.client.setHeader(
             "Access-Control-Allow-Methods",
-            "HEAD, OPTIONS, GET, PUT, DELETE, PATCH"
+            "HEAD, OPTIONS, GET, POST, PUT, DELETE, PATCH"
         )
         # Is there an input.value with format json data?
         # If so turn it into an object that emulates enough
@@ -2076,7 +2238,8 @@ def dispatch(self, method, uri, input):
         except NotFound as msg:
             output = self.error_obj(404, msg)
         except Reject as msg:
-            output = self.error_obj(405, msg)
+            output = self.error_obj(405, msg.args[0])
+            self.client.setHeader("Allow", msg.args[1])
 
         # Format the content type
         if data_type.lower() == "json":

test/rest_common.py

@@ -2062,17 +2062,17 @@ def testDispatch(self):
               ],
               "default_version": 1,
               "links": [
-                  {
-                      "rel": "summary",
-                      "uri": "http://tracker.example/cgi-bin/roundup.cgi/bugs/rest/summary"
-                  },
                   {
                       "rel": "self",
                       "uri": "http://tracker.example/cgi-bin/roundup.cgi/bugs/rest"
                   },
                   {
                       "rel": "data",
                       "uri": "http://tracker.example/cgi-bin/roundup.cgi/bugs/rest/data"
+                  },
+                  {
+                      "rel": "summary",
+                      "uri": "http://tracker.example/cgi-bin/roundup.cgi/bugs/rest/summary"
                   }
               ]
           }