launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/inspect-api-handler-uri into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~allenap/maas/inspect-api-handler-uri/+merge/124156


-- 
https://code.launchpad.net/~allenap/maas/inspect-api-handler-uri/+merge/124156
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/inspect-api-handler-uri into lp:maas.

=== modified file 'src/maasserver/apidoc.py'
--- src/maasserver/apidoc.py	2012-09-12 16:00:52 +0000
+++ src/maasserver/apidoc.py	2012-09-13 11:03:19 +0000
@@ -18,7 +18,6 @@
 
 from inspect import getdoc
 from itertools import izip_longest
-from urllib import quote
 from urlparse import urljoin
 
 from django.conf import settings
@@ -82,15 +81,31 @@
         yield next(value for value in values if value is not undefined)
 
 
+def describe_actions(handler):
+    """Describe the actions that `handler` supports."""
+    from maasserver.api import dispatch_methods  # Avoid circular imports.
+    operation_methods = getattr(handler, "_available_api_methods", {})
+    for http_method in handler.allowed_methods:
+        desc_base = dict(method=http_method)
+        if http_method in operation_methods:
+            # Default Piston CRUD method has been overridden; inspect
+            # custom operations instead.
+            operations = handler._available_api_methods[http_method]
+            for op, func in operations.items():
+                yield dict(desc_base, doc=getdoc(func), op=op, rest=False)
+        else:
+            # Default Piston CRUD method still stands.
+            op = dispatch_methods[http_method]
+            func = getattr(handler, op)
+            yield dict(desc_base, doc=getdoc(func), op=op, rest=True)
+
+
 def describe_handler(handler):
     """Return a serialisable description of a handler.
 
     :type handler: :class:`BaseHandler` instance that has been decorated by
         `api_operations`.
     """
-    # Avoid circular imports.
-    from maasserver.api import dispatch_methods
-
     uri_template = generate_doc(handler).resource_uri_template
     if uri_template is None:
         uri_template = settings.DEFAULT_MAAS_URL
@@ -103,36 +118,10 @@
         "Resource URI specifications with keyword parameters are not yet "
         "supported: handler=%r; view_name=%r" % (handler, view_name))
 
-    actions = []
-    operation_methods = getattr(handler, "_available_api_methods", {})
-    for http_method in handler.allowed_methods:
-        if http_method in operation_methods:
-            # Default Piston CRUD method has been overridden; inspect
-            # custom operations instead.
-            operations = handler._available_api_methods[http_method]
-            for op, func in operations.items():
-                desc = {
-                    "doc": getdoc(func),
-                    "method": http_method,
-                    "op": op,
-                    "uri": "%s?op=%s" % (uri_template, quote(op)),
-                    "uri_params": uri_params,
-                    }
-                actions.append(desc)
-        else:
-            # Default Piston CRUD method still stands.
-            op = dispatch_methods[http_method]
-            func = getattr(handler, op)
-            desc = {
-                "doc": getdoc(func),
-                "method": http_method,
-                "uri": uri_template,
-                "uri_params": uri_params,
-                }
-            actions.append(desc)
-
     return {
+        "actions": list(describe_actions(handler)),
+        "doc": getdoc(handler),
         "name": handler.__name__,
-        "doc": getdoc(handler),
-        "actions": actions,
+        "params": uri_params,
+        "uri": uri_template,
         }

=== modified file 'src/maasserver/tests/test_apidoc.py'
--- src/maasserver/tests/test_apidoc.py	2012-09-12 16:35:34 +0000
+++ src/maasserver/tests/test_apidoc.py	2012-09-13 11:03:19 +0000
@@ -159,24 +159,25 @@
             {"doc": "Released 1988.",
              "method": "GET",
              "op": "so_far_so_good_so_what",
-             "uri": "http://example.com/?op=so_far_so_good_so_what";,
-             "uri_params": ["vic", "rattlehead"]},
+             "rest": False},
             {"doc": "Released 1986.",
              "method": "POST",
              "op": "peace_sells_but_whos_buying",
-             "uri": "http://example.com/?op=peace_sells_but_whos_buying";,
-             "uri_params": ["vic", "rattlehead"]},
+             "rest": False},
             {"doc": None,
              "method": "PUT",
-             "uri": "http://example.com/";,
-             "uri_params": ["vic", "rattlehead"]},
+             "op": "update",
+             "rest": True},
             ]
 
         observed = describe_handler(MegadethHandler)
-        # The description contains `uri` and `actions` entries.
-        self.assertSetEqual({"actions", "doc", "name"}, set(observed))
+        # The description contains several entries.
+        self.assertSetEqual(
+            {"actions", "doc", "name", "params", "uri"},
+            set(observed))
+        self.assertEqual(MegadethHandler.__doc__, observed["doc"])
         self.assertEqual(MegadethHandler.__name__, observed["name"])
-        self.assertEqual(MegadethHandler.__doc__, observed["doc"])
+        self.assertEqual(["vic", "rattlehead"], observed["params"])
         self.assertSequenceEqual(expected_actions, observed["actions"])
 
     def test_describe_handler_with_maas_handler(self):
@@ -185,18 +186,23 @@
         from maasserver.api import NodeHandler as handler
         description = describe_handler(handler)
         # The RUD of CRUD actions are still available, but the C(reate) action
-        # has been overridden with custom operations. Not entirely sure how
-        # this makes sense, but there we go :)
+        # has been overridden with custom non-ReST operations.
         expected_actions = {
-            "DELETE http://example.com/api/1.0/nodes/{system_id}/";,
-            "GET http://example.com/api/1.0/nodes/{system_id}/";,
-            "POST http://example.com/api/1.0/nodes/{system_id}/?op=release";,
-            "POST http://example.com/api/1.0/nodes/{system_id}/?op=start";,
-            "POST http://example.com/api/1.0/nodes/{system_id}/?op=stop";,
-            "PUT http://example.com/api/1.0/nodes/{system_id}/";,
+            "DELETE op=delete rest=True",
+            "GET op=read rest=True",
+            "POST op=start rest=False",
+            "POST op=stop rest=False",
+            "POST op=release rest=False",
+            "PUT op=update rest=True",
             }
         observed_actions = {
-            "%(method)s %(uri)s" % action
+            "%(method)s op=%(op)s rest=%(rest)s" % action
             for action in description["actions"]
             }
         self.assertSetEqual(expected_actions, observed_actions)
+        self.assertSetEqual({"system_id"}, set(description["params"]))
+        # The URI is a URI Template <http://tools.ietf.org/html/rfc6570>, the
+        # components of which correspond to the parameters declared.
+        self.assertEqual(
+            "http://example.com/api/1.0/nodes/{system_id}/";,
+            description["uri"])