Merge lp:~leonardr/lazr.restful/web-link-wadl into lp:lazr.restful

[Leonard Richardson]

My earlier lazr.restful branches added a web_link field to certain representations of entries. This branch finishes off the feature by adding the web_link field to the WADL description of every entry that supports it.

The changes to webservice.txt are necessary, even though they don't change the test, because WADL generation will no longer succeed unless every entry class has a publish_web_link annotation.

The test_wadl_includes_web_link_when_available test is a little awkward, but it was the best I could do within a unit test without writing a whole lot of code. Using wadllib didn't work, because it requires a WADL file that's fully navigable starting from the service root. The stub web services provided by WebServiceTestCase don't allow that.

[Graham Binns (gmb)]

Hi Leonard,

Thanks for this branch. I found one small problem with it, which we discussed on IRC:

[15:42] gmb:
leonardr: In your diff it looks like there's a missing or extra single quote:
[15:43] gmb:
113+ <param style="plain" name="web_link" path="$[web_link']"
[15:43] gmb:
(specifically $[web_link'])
[15:43] leonardr:
gmb, thanks

Also, some of the formatting in the doctest was a bit confusing to me, so I've produced a patch that should take care of it (you don't have to apply this to land it, it's just a suggestion): http://pastebin.ubuntu.com/558144/plain/.

review: Approve (code)

=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py	2011-01-24 17:28:14 +0000
+++ src/lazr/restful/_resource.py	2011-01-25 15:32:11 +0000
@@ -1415,12 +1415,10 @@
         """
         data = {}
         data['self_link'] = absoluteURL(self.context, self.request)
-        browser_request = queryAdapter(
-            self.request, IWebBrowserOriginatingRequest)
-        if (browser_request is not None
-            and self.adapter_utility.publish_web_link):
-            # Objects in the web server correspond to objects on some website.
-            # Provide the link to the correspnding object on the website.
+        if self.adapter_utility.publish_web_link:
+            # Objects in the web service correspond to pages on some website.
+            # Provide the link to the corresponding page on the website.
+            browser_request = IWebBrowserOriginatingRequest(self.request)
             data['web_link'] = absoluteURL(self.context, browser_request)
         data['resource_type_link'] = self.type_url
         unmarshalled_field_values = {}
@@ -2156,8 +2154,14 @@
 
     @property
     def publish_web_link(self):
-        """Should this object type have a web_link published?"""
-        return self._get_tagged_value('publish_web_link')
+        """Return true if this entry should have a web_link."""
+        # If we can't adapt a web service request to a website
+        # request, we shouldn't publish a web_link for *any* entry.
+        web_service_request = get_current_web_service_request()
+        website_request = queryAdapter(
+            web_service_request, IWebBrowserOriginatingRequest)
+        return website_request is not None and self._get_tagged_value(
+            'publish_web_link')
 
     @property
     def singular_type(self):
 
=== modified file 'src/lazr/restful/docs/webservice.txt'
--- src/lazr/restful/docs/webservice.txt	2011-01-21 21:12:17 +0000
+++ src/lazr/restful/docs/webservice.txt	2011-01-25 15:32:11 +0000
@@ -555,13 +555,14 @@
     >>> from lazr.restful.interfaces import IEntry, LAZR_WEBSERVICE_NAME
     >>> class IAuthorEntry(IAuthor, IEntry):
     ...     """The part of an author we expose through the web service."""
-    ...     taggedValue(LAZR_WEBSERVICE_NAME, dict(singular="author",
-    ...                                            plural="authors"))
+    ...     taggedValue(LAZR_WEBSERVICE_NAME, dict(
+    ...         singular="author", plural="authors", publish_web_link=True))
 
     >>> class ICommentEntry(IComment, IEntry):
     ...     """The part of a comment we expose through the web service."""
     ...     taggedValue(LAZR_WEBSERVICE_NAME,
-    ...         dict(singular="comment", plural="comments"))
+    ...         dict(singular="comment", plural="comments",
+    ...              publish_web_link=True))
 
 Most of the time, it doesn't work to expose to the web service the same data
 model we expose internally. Usually there are fields we don't want to expose,
@@ -582,7 +583,8 @@
     ...     "The part of a dish that we expose through the web service."
     ...     recipes = CollectionField(value_type=Reference(schema=IRecipe))
     ...     taggedValue(LAZR_WEBSERVICE_NAME,
-    ...         dict(singular="dish", plural="dishes"))
+    ...         dict(singular="dish", plural="dishes",
+    ...         publish_web_link=True))
 
 In the following code block we define an interface that exposes the underlying
 ``Recipe``'s name but not its ID. References to associated objects (like the
@@ -596,7 +598,8 @@
     ...     instructions = Text(title=u"Name", required=True)
     ...     comments = CollectionField(value_type=Reference(schema=IComment))
     ...     taggedValue(LAZR_WEBSERVICE_NAME,
-    ...         dict(singular="recipe", plural="recipes"))
+    ...         dict(singular="recipe", plural="recipes",
+    ...         publish_web_link=True))
 
     >>> from lazr.restful.fields import ReferenceChoice
     >>> class ICookbookEntry(IEntry):
@@ -608,7 +611,8 @@
     ...     comments = CollectionField(value_type=Reference(schema=IComment))
     ...     cover = Bytes(0, 5000, title=u"An image of the cookbook's cover.")
     ...     taggedValue(LAZR_WEBSERVICE_NAME,
-    ...         dict(singular="cookbook", plural="cookbooks"))
+    ...         dict(singular="cookbook", plural="cookbooks",
+    ...         publish_web_link=True))
 
 The ``author`` field is a choice between ``Author`` objects. To make sure
 that the ``Author`` objects are properly marshalled to JSON, we need to
 
=== modified file 'src/lazr/restful/tales.py'
--- src/lazr/restful/tales.py	2011-01-21 21:12:17 +0000
+++ src/lazr/restful/tales.py	2011-01-25 15:32:11 +0000
@@ -441,6 +441,10 @@
         return self.utility.entry_page_representation_id
 
     @property
+    def publish_web_link(self):
+        return self.utility.publish_web_link
+
+    @property
     def all_fields(self):
         "Return all schema fields for the object."
         return [field for name, field in
 
=== modified file 'src/lazr/restful/templates/wadl-root.pt'
--- src/lazr/restful/templates/wadl-root.pt	2010-08-09 20:05:08 +0000
+++ src/lazr/restful/templates/wadl-root.pt	2011-01-25 15:32:11 +0000
@@ -223,6 +223,13 @@
         <wadl:doc>The canonical link to this resource.</wadl:doc>
         <link tal:attributes="resource_type context/wadl_entry:type_link" />
       </param>
+      <param style="plain" name="web_link" path="$[web_link']"
+             tal:condition="context/wadl_entry:publish_web_link">
+        <wadl:doc>
+          The canonical human-addressable web link to this resource.
+        </wadl:doc>
+        <link />
+      </param>
       <param style="plain" name="resource_type_link"
              path="$['resource_type_link']">
         <wadl:doc>
 
=== modified file 'src/lazr/restful/tests/test_webservice.py'
--- src/lazr/restful/tests/test_webservice.py	2011-01-24 22:00:56 +0000
+++ src/lazr/restful/tests/test_webservice.py	2011-01-25 15:32:11 +0000
@@ -6,6 +6,7 @@
 
 from contextlib import contextmanager
 from cStringIO import StringIO
+from lxml import etree
 from operator import attrgetter
 from textwrap import dedent
 import random
@@ -24,6 +25,8 @@
     )
 from zope.traversing.browser.interfaces import IAbsoluteURL
 
+from wadllib.application import Application
+
 from lazr.enum import EnumeratedType, Item
 from lazr.restful import ResourceOperation
 from lazr.restful.fields import Reference
@@ -126,6 +129,8 @@
 class EntryTestCase(WebServiceTestCase):
     """A test suite that defines an entry class."""
 
+    WADL_NS = "{http://research.sun.com/wadl/2006/10}"
+
     class DummyWebsiteRequest:
         """A request to the website, as opposed to the web service."""
         implements(IWebBrowserOriginatingRequest)
@@ -135,20 +140,29 @@
         URL = 'http://www.website.url/'
 
     @contextmanager
+    def request(self):
+        request = getUtility(IWebServiceConfiguration).createRequest(
+            StringIO(""), {})
+        newInteraction(request)
+        yield request
+        endInteraction()
+
+    @property
+    def wadl(self):
+        """Get a parsed WADL description of the web service."""
+        with self.request() as request:
+            return request.publication.application.toWADL().encode('utf-8')
+
+    @contextmanager
     def entry_resource(self, entry_interface, entry_implementation):
         """Create a request to an entry resource, and yield the resource."""
         entry_class = get_resource_factory(entry_interface, IEntry)
-        request = getUtility(IWebServiceConfiguration).createRequest(
-            StringIO(""), {})
-        newInteraction(request)
-
         data_object = entry_implementation("")
-        entry = entry_class(data_object, request)
-        resource = EntryResource(data_object, request)
-
-        yield resource
-
-        endInteraction()
+
+        with self.request() as request:
+            entry = entry_class(data_object, request)
+            resource = EntryResource(data_object, request)
+            yield resource
 
     def _register_url_adapter(self, entry_interface):
         """Register an IAbsoluteURL implementation for an interface."""
@@ -214,6 +228,26 @@
             self.assertEquals(
                 representation['web_link'], self.DummyWebsiteURL.URL)
 
+    def test_wadl_includes_web_link_when_available(self):
+        # If an entry includes a web_link, this information will
+        # show up in the WADL description of the entry.
+        service_root = "https://webservice_test/2.0/"
+        self._register_website_url_space(IHasOneField)
+
+        doc = etree.parse(StringIO(self.wadl))
+        # Verify that the 'has_one_field-full' representation includes
+        # a 'web_link' param.
+        representation = [
+            rep for rep in doc.findall('%srepresentation' % self.WADL_NS)
+            if rep.get('id') == 'has_one_field-full'][0]
+        param = [
+            param for param in representation.findall(
+                '%sparam' % self.WADL_NS)
+            if param.get('name') == 'web_link'][0]
+
+        # Verify that the 'web_link' param includes a 'link' tag.
+        self.assertFalse(param.find('%slink' % self.WADL_NS) is None)
+
     def test_entry_omits_web_link_when_not_available(self):
         # When there is no way of turning a webservice request into a
         # website request, the 'web_link' attribute is missing from
@@ -226,6 +260,13 @@
                 representation['self_link'], DummyAbsoluteURL.URL)
             self.assertFalse('web_link' in representation)
 
+    def test_wadl_omits_web_link_when_not_available(self):
+        # When there is no way of turning a webservice request into a
+        # website request, the 'web_link' attribute is missing from
+        # WADL descriptions of entries.
+        self._register_url_adapter(IHasOneField)
+        self.assertFalse('web_link' in self.wadl)
+
 
 class IHasNoWebLink(Interface):
     """An entry that does not publish a web_link."""