launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13070
[Merge] lp:~allenap/maas/walk-django-resolver into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/walk-django-resolver into lp:maas.
Commit message:
Walks the Django resolver configuration to find out the exposed API resources.
Previously the maasserver.api module was scanned, but this does not necessarily correspond to exposed methods.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1058316 in MAAS: "maas-cli reports unknown operation if you're logged in with the wrong credentials"
https://bugs.launchpad.net/maas/+bug/1058316
For more details, see:
https://code.launchpad.net/~allenap/maas/walk-django-resolver/+merge/128359
First branch of several.
--
https://code.launchpad.net/~allenap/maas/walk-django-resolver/+merge/128359
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/walk-django-resolver into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-10-05 08:04:58 +0000
+++ src/maasserver/api.py 2012-10-06 17:03:24 +0000
@@ -112,8 +112,8 @@
from formencode import validators
from formencode.validators import Invalid
from maasserver.apidoc import (
- describe_handler,
- find_api_handlers,
+ describe_resource,
+ find_api_resources,
generate_api_docs,
)
from maasserver.components import (
@@ -1501,6 +1501,8 @@
:return: Documentation, in ReST, for the API.
:rtype: :class:`unicode`
"""
+ from maasserver import urls_api as urlconf
+
module = sys.modules[__name__]
output = StringIO()
line = partial(print, file=output)
@@ -1512,8 +1514,8 @@
line('----------')
line()
- handlers = find_api_handlers(module)
- for doc in generate_api_docs(handlers):
+ resources = find_api_resources(urlconf)
+ for doc in generate_api_docs(resources):
uri_template = doc.resource_uri_template
exports = doc.handler.exports.items()
for (http_method, operation), function in sorted(exports):
@@ -1719,14 +1721,16 @@
Returns a JSON object describing the whole MAAS API.
"""
- module = sys.modules[__name__]
+ from maasserver import urls_api as urlconf
description = {
"doc": "MAAS API",
- "handlers": [
- describe_handler(handler)
- for handler in find_api_handlers(module)
+ "resources": [
+ describe_resource(resource)
+ for resource in find_api_resources(urlconf)
],
}
+ # For backward compatibility, add "handlers" as an alias for "resources".
+ description["handlers"] = description["resources"]
return HttpResponse(
json.dumps(description),
content_type="application/json")
=== modified file 'src/maasserver/apidoc.py'
--- src/maasserver/apidoc.py 2012-09-26 21:57:08 +0000
+++ src/maasserver/apidoc.py 2012-10-06 17:03:24 +0000
@@ -12,7 +12,8 @@
__metaclass__ = type
__all__ = [
"describe_handler",
- "find_api_handlers",
+ "describe_resource",
+ "find_api_resources",
"generate_api_docs",
]
@@ -21,36 +22,53 @@
from urlparse import urljoin
from django.conf import settings
+from django.core.urlresolvers import (
+ get_resolver,
+ RegexURLPattern,
+ RegexURLResolver,
+ )
from piston.doc import generate_doc
-from piston.handler import HandlerMetaClass
-
-
-def find_api_handlers(module):
- """Find the API handlers defined in `module`.
+from piston.handler import BaseHandler
+from piston.resource import Resource
+
+
+def accumulate_api_resources(resolver, accumulator):
+ """Accumulate handlers from the given resolver.
Handlers are of type :class:`HandlerMetaClass`, and must define a
`resource_uri` method.
:rtype: Generator, yielding handlers.
"""
- try:
- names = module.__all__
- except AttributeError:
- names = sorted(
- name for name in dir(module)
- if not name.startswith("_"))
- for name in names:
- candidate = getattr(module, name)
- if isinstance(candidate, HandlerMetaClass):
- if getattr(candidate, "resource_uri", None) is not None:
- yield candidate
-
-
-def generate_api_docs(handlers):
+ p_has_resource_uri = lambda resource: (
+ getattr(resource.handler, "resource_uri", None) is not None)
+ for pattern in resolver.url_patterns:
+ if isinstance(pattern, RegexURLResolver):
+ accumulate_api_resources(pattern, accumulator)
+ elif isinstance(pattern, RegexURLPattern):
+ if isinstance(pattern.callback, Resource):
+ resource = pattern.callback
+ if p_has_resource_uri(resource):
+ accumulator.add(resource)
+ else:
+ raise AssertionError(
+ "Not a recognised pattern or resolver: %r" % (pattern,))
+
+
+def find_api_resources(urlconf=None):
+ """Find the API resources defined in `urlconf`.
+
+ :rtype: :class:`set` of :class:`Resource` instances.
+ """
+ resolver, accumulator = get_resolver(urlconf), set()
+ accumulate_api_resources(resolver, accumulator)
+ return accumulator
+
+
+def generate_api_docs(resources):
"""Generate ReST documentation objects for the ReST API.
- Yields Piston Documentation objects describing the current registered
- handlers.
+ Yields Piston Documentation objects describing the given resources.
This also ensures that handlers define 'resource_uri' methods. This is
easily forgotten and essential in order to generate proper documentation.
@@ -58,7 +76,8 @@
:return: Generates :class:`piston.doc.HandlerDocumentation` instances.
"""
sentinel = object()
- for handler in handlers:
+ for resource in resources:
+ handler = type(resource.handler)
if getattr(handler, "resource_uri", sentinel) is sentinel:
raise AssertionError(
"Missing resource_uri in %s" % handler.__name__)
@@ -111,8 +130,12 @@
"""Return a serialisable description of a handler.
:type handler: :class:`OperationsHandler` or
- :class:`AnonymousOperationsHandler` instance.
+ :class:`AnonymousOperationsHandler` instance or subclass.
"""
+ # Want the class, not an instance.
+ if isinstance(handler, BaseHandler):
+ handler = type(handler)
+
uri_template = generate_doc(handler).resource_uri_template
if uri_template is None:
uri_template = settings.DEFAULT_MAAS_URL
@@ -132,3 +155,11 @@
"params": uri_params,
"uri": uri_template,
}
+
+
+def describe_resource(resource):
+ """Return a serialisable description of a resource.
+
+ :type resource: :class:`OperationsResource` instance.
+ """
+ return describe_handler(resource.handler)
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-10-05 08:04:58 +0000
+++ src/maasserver/tests/test_api.py 2012-10-06 17:03:24 +0000
@@ -3821,5 +3821,6 @@
def test_describe(self):
response = self.client.get(reverse('describe'))
description = json.loads(response.content)
- self.assertSetEqual({"doc", "handlers"}, set(description))
+ self.assertSetEqual(
+ {"doc", "handlers", "resources"}, set(description))
self.assertIsInstance(description["handlers"], list)
=== modified file 'src/maasserver/tests/test_apidoc.py'
--- src/maasserver/tests/test_apidoc.py 2012-09-26 23:21:46 +0000
+++ src/maasserver/tests/test_apidoc.py 2012-10-06 17:03:24 +0000
@@ -15,23 +15,32 @@
import new
from django.conf import settings
+from django.conf.urls import (
+ include,
+ patterns,
+ url,
+ )
+from django.core.exceptions import ImproperlyConfigured
from maasserver.api import (
operation,
OperationsHandler,
+ OperationsResource,
)
from maasserver.apidoc import (
describe_handler,
- find_api_handlers,
+ describe_resource,
+ find_api_resources,
generate_api_docs,
)
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
from piston.doc import HandlerDocumentation
from piston.handler import BaseHandler
-
-
-class TestFindingHandlers(TestCase):
- """Tests for API inspection support: finding handlers."""
+from piston.resource import Resource
+
+
+class TestFindingResources(TestCase):
+ """Tests for API inspection support: finding resources."""
@staticmethod
def make_module():
@@ -39,98 +48,116 @@
name = factory.make_name("module").encode("ascii")
return new.module(name)
- def test_empty_module(self):
- # No handlers are found in empty modules.
- module = self.make_module()
- module.__all__ = []
- self.assertSequenceEqual(
- [], list(find_api_handlers(module)))
-
- def test_empty_module_without_all(self):
- # The absence of __all__ does not matter.
- module = self.make_module()
- self.assertSequenceEqual(
- [], list(find_api_handlers(module)))
-
- def test_ignore_non_handlers(self):
- # Module properties that are not handlers are ignored.
- module = self.make_module()
- module.something = 123
- self.assertSequenceEqual(
- [], list(find_api_handlers(module)))
-
- def test_module_with_incomplete_handler(self):
- # Handlers that don't have a resource_uri method are ignored.
- module = self.make_module()
- module.handler = BaseHandler
- self.assertSequenceEqual(
- [], list(find_api_handlers(module)))
-
- def test_module_with_handler(self):
- # Handlers with resource_uri attributes are discovered in a module and
- # returned. The type of resource_uri is not checked; it must only be
- # present and not None.
- module = self.make_module()
- module.handler = type(
- b"MetalHander", (BaseHandler,), {"resource_uri": True})
- self.assertSequenceEqual(
- [module.handler], list(find_api_handlers(module)))
-
- def test_module_with_handler_not_in_all(self):
- # When __all__ is defined, only the names it defines are searched for
- # handlers.
- module = self.make_module()
- module.handler = BaseHandler
- module.something = "abc"
- module.__all__ = ["something"]
- self.assertSequenceEqual(
- [], list(find_api_handlers(module)))
+ def test_urlpatterns_empty(self):
+ # No resources are found in empty modules.
+ module = self.make_module()
+ module.urlpatterns = patterns("")
+ self.assertSetEqual(set(), find_api_resources(module))
+
+ def test_urlpatterns_not_present(self):
+ # The absence urlpatterns is an error.
+ module = self.make_module()
+ self.assertRaises(ImproperlyConfigured, find_api_resources, module)
+
+ def test_urlpatterns_with_resource_for_incomplete_handler(self):
+ # Resources for handlers that don't specify resource_uri are ignored.
+ module = self.make_module()
+ module.urlpatterns = patterns("", url("^foo", BaseHandler))
+ self.assertSetEqual(set(), find_api_resources(module))
+
+ def test_urlpatterns_with_resource(self):
+ # Resources for handlers with resource_uri attributes are discovered
+ # in a urlconf module and returned. The type of resource_uri is not
+ # checked; it must only be present and not None.
+ handler = type(b"\m/", (BaseHandler,), {"resource_uri": True})
+ resource = Resource(handler)
+ module = self.make_module()
+ module.urlpatterns = patterns("", url("^metal", resource))
+ self.assertSetEqual({resource}, find_api_resources(module))
+
+ def test_nested_urlpatterns_with_handler(self):
+ # Resources are found in nested urlconfs.
+ handler = type(b"\m/", (BaseHandler,), {"resource_uri": True})
+ resource = Resource(handler)
+ module = self.make_module()
+ submodule = self.make_module()
+ submodule.urlpatterns = patterns("", url("^metal", resource))
+ module.urlpatterns = patterns("", ("^genre/", include(submodule)))
+ self.assertSetEqual({resource}, find_api_resources(module))
+
+ def test_smoke(self):
+ # Resources are found for the MAAS API.
+ from maasserver import urls_api as urlconf
+ self.assertNotEqual(set(), find_api_resources(urlconf))
class TestGeneratingDocs(TestCase):
"""Tests for API inspection support: generating docs."""
@staticmethod
- def make_handler():
+ def make_resource():
"""
- Return a new `BaseHandler` subclass with a fabricated name and a
- `resource_uri` class-method.
+ Return a new `OperationsResource` with a `BaseHandler` subclass
+ handler, with a fabricated name and a `resource_uri` class-method.
"""
name = factory.make_name("handler").encode("ascii")
resource_uri = lambda cls: factory.make_name("resource-uri")
namespace = {"resource_uri": classmethod(resource_uri)}
- return type(name, (BaseHandler,), namespace)
+ handler = type(name, (BaseHandler,), namespace)
+ return OperationsResource(handler)
def test_generates_doc_for_handler(self):
# generate_api_docs() yields HandlerDocumentation objects for the
# handlers passed in.
- handler = self.make_handler()
- docs = list(generate_api_docs([handler]))
+ resource = self.make_resource()
+ docs = list(generate_api_docs([resource]))
self.assertEqual(1, len(docs))
[doc] = docs
self.assertIsInstance(doc, HandlerDocumentation)
- self.assertIs(handler, doc.handler)
+ self.assertIs(type(resource.handler), doc.handler)
def test_generates_doc_for_multiple_handlers(self):
# generate_api_docs() yields HandlerDocumentation objects for the
# handlers passed in.
- handlers = [self.make_handler() for _ in range(5)]
- docs = list(generate_api_docs(handlers))
- self.assertEqual(len(handlers), len(docs))
- self.assertEqual(handlers, [doc.handler for doc in docs])
+ resources = [self.make_resource() for _ in range(5)]
+ docs = list(generate_api_docs(resources))
+ self.assertEqual(len(resources), len(docs))
+ self.assertEqual(
+ [type(resource.handler) for resource in resources],
+ [doc.handler for doc in docs])
def test_handler_without_resource_uri(self):
# generate_api_docs() raises an exception if a handler does not have a
# resource_uri attribute.
- handler = self.make_handler()
- del handler.resource_uri
- docs = generate_api_docs([handler])
+ resource = self.make_resource()
+ del type(resource.handler).resource_uri
+ docs = generate_api_docs([resource])
error = self.assertRaises(AssertionError, list, docs)
self.assertEqual(
- "Missing resource_uri in %s" % handler.__name__,
+ "Missing resource_uri in %s" % type(resource.handler).__name__,
unicode(error))
+class MegadethHandler(OperationsHandler):
+ """The mighty 'deth."""
+
+ create = read = delete = None
+
+ @operation(idempotent=False)
+ def peace_sells_but_whos_buying(self, request, vic, rattlehead):
+ """Released 1986."""
+
+ @operation(idempotent=True)
+ def so_far_so_good_so_what(self, request, vic, rattlehead):
+ """Released 1988."""
+
+ @classmethod
+ def resource_uri(cls):
+ # Note that the arguments, after request, to each of the ops
+ # above matches the parameters (index 1) in the tuple below.
+ return ("megadeth_view", ["vic", "rattlehead"])
+
+
class TestDescribingAPI(TestCase):
"""Tests for functions that describe a Piston API."""
@@ -144,26 +171,6 @@
def test_describe_handler(self):
# describe_handler() returns a description of a handler that can be
# readily serialised into JSON, for example.
-
- class MegadethHandler(OperationsHandler):
- """The mighty 'deth."""
-
- create = read = delete = None
-
- @operation(idempotent=False)
- def peace_sells_but_whos_buying(self, request, vic, rattlehead):
- """Released 1986."""
-
- @operation(idempotent=True)
- def so_far_so_good_so_what(self, request, vic, rattlehead):
- """Released 1988."""
-
- @classmethod
- def resource_uri(cls):
- # Note that the arguments, after request, to each of the ops
- # above matches the parameters (index 1) in the tuple below.
- return ("megadeth_view", ["vic", "rattlehead"])
-
expected_actions = [
{"doc": "Released 1988.",
"method": "GET",
@@ -181,7 +188,6 @@
"op": None,
"restful": True},
]
-
observed = describe_handler(MegadethHandler)
# The description contains several entries.
self.assertSetEqual(
@@ -218,3 +224,11 @@
self.assertEqual(
"http://example.com/api/1.0/nodes/{system_id}/";,
description["uri"])
+
+ def test_describe_resource(self):
+ # describe_resource() returns a description of a resource. Right now
+ # it is just the description of the resource's handler class.
+ resource = OperationsResource(MegadethHandler)
+ self.assertEqual(
+ describe_handler(MegadethHandler),
+ describe_resource(resource))
