Merge lp:~leonardr/lazr.restful/include-html-field-representations into lp:lazr.restful

[Leonard Richardson]

This is a pretty interesting branch. It needs a follow-up to fix some edge cases, but I'm already past the 800-line limit so I'll do it afterwards.

The idea behind this branch is that by adding a parameter to the Accept header when you ask for a JSON representation of an entry, you can get a JSON representation that includes the essential portions of an HTML representation.

GET /foo
Accept: application/json;include=lp_html

200 Ok
Content-Type: application/json;include=lp_html
{"foo" : "value", "bar" : "value",
 "lp_html" : { "foo": "<b>value</b>", bar: "<a href=\"/bar\">bar</a>" }
}

That lp_html sub-dictionary contains HTML representations of any of the entry's fields that have an IFieldHTMLRenderer registered for them.

= Why? =

This is for the Launchpad website. Currently we get raw data from the JSON representation of the context, as stored in LP.client.cache['context']. When we change something, we send a PATCH request and ask Launchpad to send us an HTML representation of the new object. We use the HTML representation in an ad hoc way and then throw it away. Since we changed the object, the JSON representation is now in a known bad state, and we have no way of updating it.

We need a single representation that contains both the raw data and the HTML representations. We need a *single* representation because when we make a PATCH request the server only sends back one document describing the new object. My squad discussed several alternatives and settled on extending the application/json media type with a parameter that includes a sub-dictionary of field HTML representations.

We will not be describing the application/json;include=lp_html representation in the WADL, since we only intend to use it in an AJAX environment. If this turns out to be generally useful, we may integrate lp_html into the main application/json representation, and get rid of application/json;include=lp_html.

= How? =

1. The first big chunk of code to look at is parse_accept_style_header, in test_utils.py, at the end of the diff. I moved a private method out of resource.py and generalized the implementation. The basic idea is that an Accept header comes in with a bunch of media types, each of which may have a bunch of parameters including a "q" parameter:

application/json;q=0.7;include=lp_html, application/json;q=0.6, text/html;q=0.1

The goal of parse_accept_style_header is to use that "q" parameter to determine the client's preferred media type. In this case, the client would prefer application/json;include=lp_html (q=0.7), and if that's not available would prefer application/json (q=0.6), and if neither is available will settle for text/html (q=0.1).

Unfortunately, the previous implementation of parse_accept_style_header didn't work if the Accept header mentioned parameters other than 'q'. I made the code more general and added a lot of tests. Now parse_accept_style_header consumes the 'q' parameter, leaving the other parameters alone.

2. I started passing in a 'media_type' argument to simplejson.dumps(). This is passed into the ResourceJSONEncoder constructor. This makes it all the way into EntryResource.toDataStructure, which builds the 'lp_html' dictionary and adds it to the main dictionary if it turns out not to be empty.

3. The rest of this diff is tests. It's mostly unit tests but I added a doctest to demonstrate the feature in a human-readable way.

I did a drive-by cleanup of a number of the tests in test_webservice (HTMLRepresentationTest and UnicodeErrorTestCase), making them subclass EntryTestCase and having them take advantage of the context managers.

[j.c.sackett (jcsackett)]

Leonard--

This looks good. Massive, but good. I've got some comments in the diff below, but they're not landing blockers.

> === modified file 'src/lazr/restful/_resource.py'
> --- src/lazr/restful/_resource.py 2011-02-01 16:40:16 +0000
> +++ src/lazr/restful/_resource.py 2011-02-17 15:42:47 +0000
>
> @@ -1165,8 +1140,9 @@
> # fields that don't correspond to some field in the
> # schema. They're all errors.
> for invalid_field in changeset.keys():
> - errors.append(u"%s: You tried to modify a nonexistent "
> - "attribute." % invalid_field)
> + if not invalid_field.endswith('_html'):
> + errors.append(u"%s: You tried to modify a nonexistent "
> + "attribute." % invalid_field)

Just commenting that you already told me you're improving this in the followup, since others may look at this MP. I agree that it's not the right way to do this, and I'm glad you're fixing it in a subsequent branch.

> === modified file 'src/lazr/restful/utils.py'
> --- src/lazr/restful/utils.py 2010-09-28 12:14:12 +0000
> +++ src/lazr/restful/utils.py 2011-02-17 15:42:47 +0000
> return versions.index(total_size_link_version) <= versions.index(version)
>
>
> +def parse_accept_style_header(value):
> + """Parse an HTTP header from the Accept-* family.
> +
> + These headers contain a list of possible values, each with an
> + optional priority.
> +
> + This code is modified from Zope's
> + BrowserLanguages#getPreferredLanguages.
> +
> + If a value includes parameters other than 'q', they will be preserved.
> +
> + :return: All values, in descending order of priority.
> + """
> + if value is None:
> + return []
> +
> + values = value.split(',')
> + # In the original getPreferredLanguages there was some language
> + # code normalization here, which I removed.

I don't know that this comment is needed; it doesn't really make sense when looking through it now. It seems like the sort of thing needed in commit history or MPs, but not code.

> + values = [v.strip() for v in values if v != ""]
> +
> + accepts = []
> + for index, value in enumerate(values):
> + value_and_parameters = value.split(';')
> +
> + # If not supplied, quality defaults to 1.
> + quality = 1.0
> +
> + if len(value_and_parameters) > 1:
> + # There's at least one parameter, possibly including
> + # a quality parameter.
> + quality_parameter = None
> + bare_value = value_and_parameters[0]
> + parameters = value_and_parameters[1:]
> + for parameter_index, parameter in enumerate(parameters):
> + if parameter.startswith('q='):
> + # We found the quality parameter
> + quality_parameter = parameter
> + q = quality_parameter.split('=', 2)[1]
> + quality = float(q)
> +
> + if quality_parameter is not None:
> + # Remove the quality parameter from the list of
> + # parameters, and re-create the header value without
> ...

review: Approve (code*)

[lp:~leonardr/lazr.restful/include-html-field-representations]

183. By Leonard Richardson on 2011-02-17

Minor cleanup in response to feedback.

[Curtis Hovey (sinzui)]

Hi Leonard.

I am approving this branch because I think you have a good understanding of the issues here and the code is a good read. I have a few remarks/questions about lines I would have done differently because of issues I have seen while fixing bugs. The issues are trivial to fix if they exist and I am sure you can judge if you need to make them

=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py 2011-02-01 16:40:16 +0000
+++ src/lazr/restful/_resource.py 2011-02-17 19:35:43 +0000
...
> @@ -163,6 +164,21 @@
> return str(value)
>
>
> +def _default_html_renderer(value):
> + """The default technique for rendering a value as an HTML snippet."""
> + return cgi.escape(unicode(value))

Is there any chance that the value could be a translatable message or a
structured string? If so, I think this should be
canonical.launchpad.webapp.menu.escape()
...

> @@ -1359,7 +1336,8 @@
> SUPPORTED_CONTENT_TYPES = [HTTPResource.WADL_TYPE,
> HTTPResource.DEPRECATED_WADL_TYPE,
> HTTPResource.XHTML_TYPE,
> - HTTPResource.JSON_TYPE]
> + HTTPResource.JSON_TYPE,
> + HTTPResource.JSON_PLUS_XHTML_TYPE]

If this list was written as PEP 8 advises, there would be less to change:
                               HTTPResource.JSON_TYPE,
                               HTTPResource.JSON_PLUS_XHTML_TYPE,
                               ]

...
> @@ -1438,9 +1416,21 @@
> data['web_link'] = absoluteURL(self.context, browser_request)
> data['resource_type_link'] = self.type_url
> unmarshalled_field_values = {}
> + html_renderings = {}
> for name, field in getFieldsInOrder(self.entry.schema):
> - if media_type == self.JSON_TYPE:
> - repr_name, repr_value = self._unmarshallField(name, field)
> + if media_type.startswith(self.JSON_TYPE):
> + repr_name, repr_value, html_renderer = (
> + self._field_with_html_renderer(name, field))
> + if media_type == self.JSON_PLUS_XHTML_TYPE and html_renderer != _default_html_renderer:

Can this line be wrapped? I cannot see it without scrolling left?
                if (media_type == self.JSON_PLUS_XHTML_TYPE
                    and html_renderer != _default_html_renderer):

> === modified file 'src/lazr/restful/tests/test_utils.py'
> --- src/lazr/restful/tests/test_utils.py 2010-09-28 12:14:12 +0000
> +++ src/lazr/restful/tests/test_utils.py 2011-02-17 19:35:43 +0000
>
> @@ -90,3 +91,52 @@
>
> # For the sake of convenience, test_get_current_web_service_request()
> # and tag_request_with_version_name() are tested in test_webservice.py.
> +
> +
> +class TestParseAcceptStyleHeader(unittest.TestCase):
> +
> + def test_single_value(self):
> + self.assertEquals(parse_accept_style_header("foo"), ["foo"])

I believe assertEquals() is being deprecated. Our style guide says to use
assertEqual().

> === modified file 'src/lazr/restful/utils.py'
> --- src/lazr/restful/utils.py 2010-09-28 12:14:12 ...

review: Approve (code)

=== modified file 'src/lazr/restful/NEWS.txt'
--- src/lazr/restful/NEWS.txt	2011-02-16 13:43:38 +0000
+++ src/lazr/restful/NEWS.txt	2011-02-17 19:35:43 +0000
@@ -2,6 +2,12 @@
 NEWS for lazr.restful
 =====================
 
+0.17.0 (2011-02-17)
+===================
+
+Added the ability to get a combined JSON/HTML representation of an
+entry that has custom HTML representations for some of its fields.
+
 0.16.1 (2011-02-16)
 ===================
 

=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py	2011-02-01 16:40:16 +0000
+++ src/lazr/restful/_resource.py	2011-02-17 19:35:43 +0000
@@ -118,6 +118,7 @@
     extract_write_portion,
     get_current_browser_request,
     get_current_web_service_request,
+    parse_accept_style_header,
     sorted_named_things,
     )
 
@@ -163,6 +164,21 @@
         return str(value)
 
 
+def _default_html_renderer(value):
+    """The default technique for rendering a value as an HTML snippet."""
+    return cgi.escape(unicode(value))
+
+
[e-mail](Interface, IField, IWebServiceClientRequest)
+@implementer(IFieldHTMLRenderer)
+def render_field_to_html(object, field, request):
+    """Turn a field's current value into an XHTML snippet.
+
+    This is the default adapter for IFieldHTMLRenderer.
+    """
+    return _default_html_renderer
+
+
 def register_versioned_request_utility(interface, version):
     """Registers a marker interface as a utility for version lookup.
 
@@ -187,6 +203,10 @@
     instance, should implement IJSONPublishable.
     """
 
+    def __init__(self, *args, **kwargs):
+        self.media_type = kwargs.pop('media_type', HTTPResource.JSON_TYPE)
+        super(ResourceJSONEncoder, self).__init__(*args, **kwargs)
+
     def default(self, obj):
         """Convert the given object to a simple data structure."""
         if isinstance(obj, datetime) or isinstance(obj, date):
@@ -209,7 +229,7 @@
         if queryMultiAdapter((obj, request), IEntry):
             obj = EntryResource(obj, request)
 
-        return IJSONPublishable(obj).toDataForJSON()
+        return IJSONPublishable(obj).toDataForJSON(self.media_type)
 
 
 class JSONItem:
@@ -220,7 +240,7 @@
     def __init__(self, context):
         self.context = context
 
-    def toDataForJSON(self):
+    def toDataForJSON(self, media_type):
         """See `ISJONPublishable`"""
         return str(self.context.title)
 
@@ -246,6 +266,7 @@
     # Some interesting media types.
     WADL_TYPE = 'application/vnd.sun.wadl+xml'
     JSON_TYPE = 'application/json'
+    JSON_PLUS_XHTML_TYPE = 'application/json;include=lp_html'
     XHTML_TYPE = 'application/xhtml+xml'
 
     # This misspelling of the WADL media type was used for a while,
@@ -257,7 +278,7 @@
     WADL_TEMPLATE = LazrPageTemplateFile('templates/wadl-resource.pt')
 
     # All resources serve WADL and JSON representations. Only entry
-    # resources serve XHTML representations.
+    # resources serve XHTML representations and JSON+XHTML representations.
     SUPPORTED_CONTENT_TYPES = [WADL_TYPE, DEPRECATED_WADL_TYPE, JSON_TYPE]
 
     def __init__(self, context, request):
@@ -472,7 +493,7 @@
         """Find which content types the client prefers to receive."""
         accept_header = (self.request.form.pop('ws.accept', None)
             or self.request.get('HTTP_ACCEPT'))
-        return self._parseAcceptStyleHeader(accept_header)
+        return parse_accept_style_header(accept_header)
 
     def _parseETags(self, header_name):
         """Extract a list of ETags from a header and parse the list.
@@ -534,52 +555,6 @@
             params[name] = value
         return (disposition, params)
 
-    def _parseAcceptStyleHeader(self, value):
-        """Parse an HTTP header from the Accept-* family.
-
-        These headers contain a list of possible values, each with an
-        optional priority.
-
-        This code is modified from Zope's
-        BrowserLanguages#getPreferredLanguages.
-
-        :return: All values, in descending order of priority.
-        """
-        if value is None:
-            return []
-
-        values = value.split(',')
-        # In the original getPreferredLanguages there was some language
-        # code normalization here, which I removed.
-        values = [v for v in values if v != ""]
-
-        accepts = []
-        for index, value in enumerate(values):
-            l = value.split(';', 2)
-
-            # If not supplied, quality defaults to 1...
-            quality = 1.0
-
-            if len(l) == 2:
-                q = l[1]
-                if q.startswith('q='):
-                    q = q.split('=', 2)[1]
-                    quality = float(q)
-
-            if quality == 1.0:
-                # ... but we use 1.9 - 0.001 * position to
-                # keep the ordering between all items with
-                # 1.0 quality, which may include items with no quality
-                # defined, and items with quality defined as 1.
-                quality = 1.9 - (0.001 * index)
-
-            accepts.append((quality, l[0].strip()))
-
-        accepts = [acc for acc in accepts if acc[0] > 0]
-        accepts.sort()
-        accepts.reverse()
-        return [value for quality, value in accepts]
-
 
 class WebServiceBatchNavigator(BatchNavigator):
     """A batch navigator that speaks to web service clients.
@@ -835,13 +810,13 @@
             self._unmarshalled_field_cache[field_name] = unmarshalled
         return unmarshalled
 
-    def unmarshallFieldToHTML(self, field_name, field):
-        """See what a field would look like in an HTML representation.
-
-        This is usually similar to the value of _unmarshallField(),
-        but it might contain some custom HTML weirdness.
-
-        :return: a 2-tuple (representation_name, representation_value).
+    def _field_with_html_renderer(self, field_name, field):
+        """Find an IFieldHTMLRenderer for the given field.
+
+        :return: A 3-tuple (name, value, renderer). 'name' and 'value'
+          are the unmarshalled field name and JSON-ready
+          value. 'renderer' is an object capable of turning the
+          JSON-ready value into an HTML value.
         """
         name, value = self._unmarshallField(field_name, field)
         try:
@@ -856,21 +831,21 @@
             renderer = getMultiAdapter(
                 (self.entry.context, field, self.request),
                 IFieldHTMLRenderer)
+        return name, value, renderer
+
+    def unmarshallFieldToHTML(self, field_name, field):
+        """See what a field would look like in an HTML representation.
+
+        This is usually similar to the value of _unmarshallField(),
+        but it might contain some custom HTML weirdness.
+
+        :return: a 2-tuple (representation_name, representation_value).
+        """
+        name, value, renderer = self._field_with_html_renderer(
+            field_name, field)
         return name, renderer(value)
 
 
[e-mail](Interface, IField, IWebServiceClientRequest)
-@implementer(IFieldHTMLRenderer)
-def render_field_to_html(object, field, request):
-    """Turn a field's current value into an XHTML snippet.
-
-    This is the default adapter for IFieldHTMLRenderer.
-    """
-    def unmarshall(value):
-        return cgi.escape(unicode(value))
-    return unmarshall
-
-
 class ReadOnlyResource(HTTPResource):
     """A resource that serves a string in response to GET."""
 
@@ -1165,8 +1140,9 @@
         # fields that don't correspond to some field in the
         # schema. They're all errors.
         for invalid_field in changeset.keys():
-            errors.append(u"%s: You tried to modify a nonexistent "
-                           "attribute." % invalid_field)
+            if not invalid_field.endswith('_html'):
+                errors.append(u"%s: You tried to modify a nonexistent "
+                              "attribute." % invalid_field)
 
         # If there were errors, display them and send a status of 400.
         if len(errors) > 0:
@@ -1239,6 +1215,7 @@
         """
         pass
 
+
 class EntryFieldResource(FieldUnmarshallerMixin, EntryManipulatingResource):
     """An individual field of an entry."""
     implements(IEntryFieldResource, IJSONPublishable)
@@ -1359,7 +1336,8 @@
     SUPPORTED_CONTENT_TYPES = [HTTPResource.WADL_TYPE,
                                HTTPResource.DEPRECATED_WADL_TYPE,
                                HTTPResource.XHTML_TYPE,
-                               HTTPResource.JSON_TYPE]
+                               HTTPResource.JSON_TYPE,
+                               HTTPResource.JSON_PLUS_XHTML_TYPE]
 
     def __init__(self, context, request):
         """Associate this resource with a specific object and request."""
@@ -1416,9 +1394,9 @@
         return existing_write_portion in incoming_write_portions
 
 
-    def toDataForJSON(self):
+    def toDataForJSON(self, media_type=None):
         """Turn the object into a simple data structure."""
-        return self.toDataStructure(self.JSON_TYPE)
+        return self.toDataStructure(media_type or self.JSON_TYPE)
 
     def toDataStructure(self, media_type):
         """Turn the object into a simple data structure.
@@ -1438,9 +1416,21 @@
             data['web_link'] = absoluteURL(self.context, browser_request)
         data['resource_type_link'] = self.type_url
         unmarshalled_field_values = {}
+        html_renderings = {}
         for name, field in getFieldsInOrder(self.entry.schema):
-            if media_type == self.JSON_TYPE:
-                repr_name, repr_value = self._unmarshallField(name, field)
+            if media_type.startswith(self.JSON_TYPE):
+                repr_name, repr_value, html_renderer = (
+                    self._field_with_html_renderer(name, field))
+                if media_type == self.JSON_PLUS_XHTML_TYPE and html_renderer != _default_html_renderer:
+                    # This field has an unusual HTML renderer.
+                    if repr_value == self.REDACTED_VALUE:
+                        # The data is redacted, so the HTML
+                        # representation of that data should also
+                        # be redacted.
+                        html_value = self.REDACTED_VALUE
+                    else:
+                        html_value = html_renderer(repr_value)
+                    html_renderings[repr_name] = html_value
             elif media_type == self.XHTML_TYPE:
                 repr_name, repr_value = self.unmarshallFieldToHTML(
                     name, field)
@@ -1451,6 +1441,8 @@
             data[repr_name] = repr_value
             unmarshalled_field_values[name] = repr_value
 
+        if len(html_renderings) > 0:
+            data['lp_html'] = html_renderings
         etag = self.getETag(media_type, unmarshalled_field_values)
         data['http_etag'] = etag
         return data
@@ -1647,19 +1639,20 @@
 
         if media_type in [self.WADL_TYPE, self.DEPRECATED_WADL_TYPE]:
             return self.toWADL().encode("utf-8")
-        elif media_type == self.JSON_TYPE:
+        elif media_type in (self.JSON_TYPE, self.JSON_PLUS_XHTML_TYPE):
             cache = self._representation_cache
             if cache is None:
                 representation = None
             else:
                 representation = cache.get(
-                    self.context, self.JSON_TYPE, self.request.version)
+                    self.context, media_type, self.request.version)
 
             redacted_fields = self.redacted_fields
             if representation is None:
                 # Either there is no active cache, or the representation
                 # wasn't in the cache.
-                representation = simplejson.dumps(self, cls=ResourceJSONEncoder)
+                representation = simplejson.dumps(
+                    self, cls=ResourceJSONEncoder, media_type=media_type)
                 # If there's an active cache, and this representation
                 # doesn't contain any redactions, store it in the
                 # cache.
@@ -1960,7 +1953,7 @@
         namespace['collections'] = sorted_named_things(collection_classes)
         return self.WADL_TEMPLATE.pt_render(namespace)
 
-    def toDataForJSON(self):
+    def toDataForJSON(self, media_type):
         """Return a map of links to top-level collection resources.
 
         A top-level resource is one that adapts a utility.  Currently

=== modified file 'src/lazr/restful/example/base/tests/field.txt'
--- src/lazr/restful/example/base/tests/field.txt	2011-02-02 17:48:38 +0000
+++ src/lazr/restful/example/base/tests/field.txt	2011-02-17 19:35:43 +0000
@@ -274,13 +274,12 @@
   ...     IFieldHTMLRenderer, IWebServiceClientRequest)
   >>> from lazr.restful.example.base.interfaces import ICookbook
 
+  >>> from lazr.restful.testing.webservice import simple_renderer
   >>> @component.adapter(ICookbook, ITextLine, IWebServiceClientRequest)
   ... @implementer(IFieldHTMLRenderer)
   ... def dummy_renderer(context, field, request):
   ...     """Bold the original string and add a snowman."""
-  ...     def render(value):
-  ...         return u"\N{SNOWMAN} <b>%s</b>" % value
-  ...     return render
+  ...     return simple_renderer
 
   >>> print webservice.get(cookbook_url +'/name', 'application/xhtml+xml')
   HTTP/1.1 200 Ok
@@ -328,6 +327,40 @@
   ...
   1995-01-01
 
+Combined JSON/HTML representations
+----------------------------------
+
+You can get a combined JSON/HTML representation of an entry by setting
+the "include=lp_html" parameter on the application/json media type.
+
+  >>> json = webservice.get(
+  ...     cookbook_url, 'application/json;include=lp_html').jsonBody()
+
+The cookbook's description is a normal JSON representation...
+
+  >>> print json['description']
+  Description
+
+...but the JSON dictionary will include a 'lp_html' sub-dictionary...
+
+  >>> html = json['lp_html']
+
+...which includes HTML representations of the fields with HTML
+representations:
+
+  >>> sorted(html.keys())
+  [u'description', u'name']
+
+  >>> from lazr.restful.testing.helpers import encode_unicode
+  >>> print encode_unicode(html['description'])
+  \u2603 <b>Description</b>
+
+  >>> print encode_unicode(html['name'])
+  \u2603 <b>The Joy of Cooking</b>
+
+Cleanup
+-------
+
 Before we continue, here's some cleanup code to remove the custom
 renderer we just defined.
 

=== modified file 'src/lazr/restful/interfaces/_rest.py'
--- src/lazr/restful/interfaces/_rest.py	2011-01-20 23:54:37 +0000
+++ src/lazr/restful/interfaces/_rest.py	2011-02-17 19:35:43 +0000
@@ -91,11 +91,15 @@
 class IJSONPublishable(Interface):
     """An object that can be published as a JSON data structure."""
 
-    def toDataForJSON():
+    def toDataForJSON(media_type):
         """Return a representation that can be turned into JSON.
 
         The representation must consist entirely of simple data
         structures and IJSONPublishable objects.
+
+        :param media_type: The media type that the data will be
+        converted to. This will be application/json, obviously, but it
+        may include parameters.
         """
 
 class IServiceRootResource(IHTTPResource):

=== modified file 'src/lazr/restful/testing/helpers.py'
--- src/lazr/restful/testing/helpers.py	2011-02-02 18:00:38 +0000
+++ src/lazr/restful/testing/helpers.py	2011-02-17 19:35:43 +0000
@@ -48,7 +48,18 @@
     :param response: an httplib HTTPResponse object.
     """
     response_unicode = str(response).decode("utf-8")
-    return response_unicode.encode("ascii", "backslashreplace")
+    return encode_unicode(response_unicode)
+
+
+def encode_unicode(unicode_string):
+    """Encode a Unicode string so it can be used in tests.
+
+    The resulting string will (eg.) contain the six characters
+    "\u2603" instead of the single Unicode character SNOWMAN.
+
+    :param unicode_string: A Unicode string.
+    """
+    return unicode_string.encode("ascii", "backslashreplace")
 
 
 class TestWebServiceConfiguration:

=== modified file 'src/lazr/restful/testing/webservice.py'
--- src/lazr/restful/testing/webservice.py	2011-02-02 13:31:21 +0000
+++ src/lazr/restful/testing/webservice.py	2011-02-17 19:35:43 +0000
@@ -14,6 +14,7 @@
     'IGenericEntry',
     'IGenericCollection',
     'pprint_entry',
+    'simple_renderer',
     'WebServiceTestCase',
     'WebServiceTestConfiguration'
     'WebServiceTestPublication',
@@ -565,3 +566,14 @@
     adapts(DummyRootResource, IWebServiceClientRequest)
 
     URL = 'http://dummyurl'
+
+
+def simple_renderer(value):
+     """Bold the original string and add a snowman.
+
+     To use this function, define an IHTMLFieldRenderer that returns it.
+
+     This is a good HTML field renderer to use in tests, because it
+     tests Unicode values and embedded HTML.
+     """
+     return u"\N{SNOWMAN} <b>%s</b>" % value

=== modified file 'src/lazr/restful/tests/test_utils.py'
--- src/lazr/restful/tests/test_utils.py	2010-09-28 12:14:12 +0000
+++ src/lazr/restful/tests/test_utils.py	2011-02-17 19:35:43 +0000
@@ -15,6 +15,7 @@
     extract_write_portion,
     get_current_browser_request,
     is_total_size_link_active,
+    parse_accept_style_header,
     sorted_named_things,
     )
 
@@ -90,3 +91,52 @@
 
     # For the sake of convenience, test_get_current_web_service_request()
     # and tag_request_with_version_name() are tested in test_webservice.py.
+
+
+class TestParseAcceptStyleHeader(unittest.TestCase):
+
+    def test_single_value(self):
+        self.assertEquals(parse_accept_style_header("foo"), ["foo"])
+
+    def test_multiple_unodered_values(self):
+        self.assertEquals(
+            parse_accept_style_header("foo, bar"),
+            ["foo", "bar"])
+
+        self.assertEquals(
+            parse_accept_style_header("foo, bar,baz"),
+            ["foo", "bar", "baz"])
+
+    def test_highest_quality_parameter_wins(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;q=0.001, bar;q=0.05, baz;q=0.1"),
+            ["baz", "bar", "foo"])
+
+    def test_quality_zero_is_omitted(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;q=0, bar;q=0.5"), ["bar"])
+
+    def test_no_quality_parameter_is_implicit_one_point_zero(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;q=0.5, bar"),
+            ["bar", "foo"])
+
+    def test_standalone_parameter_is_untouched(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;a=0.5"),
+            ["foo;a=0.5"])
+
+    def test_quality_parameter_is_removed_next_parameter_is_untouched(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;a=bar;q=0.5"),
+            ["foo;a=bar"])
+
+    def test_quality_parameter_is_removed_earlier_parameter_is_untouched(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;q=0.5;a=bar"),
+            ["foo;a=bar"])
+
+    def test_quality_parameter_is_removed_surrounding_parameters_are_untouched(self):
+        self.assertEquals(
+            parse_accept_style_header("foo;a=bar;q=0.5;b=baz"),
+            ["foo;a=bar;b=baz"])

=== modified file 'src/lazr/restful/tests/test_webservice.py'
--- src/lazr/restful/tests/test_webservice.py	2011-02-02 13:25:47 +0000
+++ src/lazr/restful/tests/test_webservice.py	2011-02-17 19:35:43 +0000
@@ -19,6 +19,7 @@
 from zope.interface.interface import InterfaceClass
 from zope.publisher.browser import TestRequest
 from zope.schema import Choice, Date, Datetime, TextLine
+from zope.schema.interfaces import ITextLine
 from zope.security.management import (
     endInteraction,
     newInteraction,
@@ -37,6 +38,7 @@
 from lazr.restful.interfaces import (
     ICollection,
     IEntry,
+    IFieldHTMLRenderer,
     IResourceGETOperation,
     IServiceRootResource,
     IWebBrowserOriginatingRequest,
@@ -56,6 +58,7 @@
     DummyAbsoluteURL,
     IGenericCollection,
     IGenericEntry,
+    simple_renderer,
     WebServiceTestCase,
     )
 from lazr.restful.testing.tales import test_tales
@@ -130,6 +133,8 @@
 
     WADL_NS = "{http://research.sun.com/wadl/2006/10}"
 
+    default_media_type = "application/json"
+
     class DummyWebsiteRequest:
         """A request to the website, as opposed to the web service."""
         implements(IWebBrowserOriginatingRequest)
@@ -139,9 +144,10 @@
         URL = 'http://www.website.url/'
 
     @contextmanager
-    def request(self):
+    def request(self, media_type=None):
+        media_type = media_type or self.default_media_type
         request = getUtility(IWebServiceConfiguration).createRequest(
-            StringIO(""), {})
+            StringIO(""), {'HTTP_ACCEPT' : media_type})
         newInteraction(request)
         yield request
         endInteraction()
@@ -153,16 +159,40 @@
             return request.publication.application.toWADL().encode('utf-8')
 
     @contextmanager
-    def entry_resource(self, entry_interface, entry_implementation):
+    def entry_resource(self, entry_interface, entry_implementation,
+                       *implementation_args):
         """Create a request to an entry resource, and yield the resource."""
         entry_class = get_resource_factory(entry_interface, IEntry)
-        data_object = entry_implementation("")
+        data_object = entry_implementation(*implementation_args)
 
         with self.request() as request:
             entry = entry_class(data_object, request)
             resource = EntryResource(data_object, request)
             yield resource
 
+    @contextmanager
+    def entry_field_resource(self, entry_interface, entry_implementation,
+                             field_name, *implementation_args):
+        entry_class = get_resource_factory(entry_interface, IEntry)
+        data_object = entry_implementation(*implementation_args)
+        with self.request() as request:
+            entry = entry_class(data_object, request)
+            field = entry.schema.get(field_name)
+            entry_field = EntryField(entry, field, field_name)
+            resource = EntryFieldResource(entry_field, request)
+            yield resource
+
+    def register_html_field_renderer(self, entry_interface, field_interface,
+                                     render_function, name=''):
+        """Register an HTML representation for a field or class of field."""
+        def renderer(context, field, request):
+            return render_function
+
+        getGlobalSiteManager().registerAdapter(
+            renderer,
+            (entry_interface, field_interface, IWebServiceClientRequest),
+            IFieldHTMLRenderer, name=name)
+
     def _register_url_adapter(self, entry_interface):
         """Register an IAbsoluteURL implementation for an interface."""
         getGlobalSiteManager().registerAdapter(
@@ -204,7 +234,22 @@
         self.a_field = value
 
 
-class TestEntryRead(EntryTestCase):
+class IHasTwoFields(Interface):
+    """An entry with two fields."""
+    export_as_webservice_entry()
+    a_field = exported(TextLine(title=u"A field."))
+    another_field = exported(TextLine(title=u"Another field."))
+
+
+class HasTwoFields:
+    """An implementation of IHasTwoFields."""
+    implements(IHasTwoFields)
+    def __init__(self, value1, value2):
+        self.a_field = value1
+        self.another_field = value2
+
+
+class TestEntryWebLink(EntryTestCase):
 
     testmodule_objects = [HasOneField, IHasOneField]
 
@@ -221,7 +266,7 @@
 
         # Now a representation of IHasOneField includes a
         # 'web_link'.
-        with self.entry_resource(IHasOneField, HasOneField) as resource:
+        with self.entry_resource(IHasOneField, HasOneField, "") as resource:
             representation = resource.toDataForJSON()
             self.assertEquals(representation['self_link'], DummyAbsoluteURL.URL)
             self.assertEquals(
@@ -253,7 +298,7 @@
         # entry representations.
         self._register_url_adapter(IHasOneField)
 
-        with self.entry_resource(IHasOneField, HasOneField) as resource:
+        with self.entry_resource(IHasOneField, HasOneField, "") as resource:
             representation = resource.toDataForJSON()
             self.assertEquals(
                 representation['self_link'], DummyAbsoluteURL.URL)
@@ -288,7 +333,8 @@
     def test_entry_omits_web_link_when_suppressed(self):
         self._register_website_url_space(IHasNoWebLink)
 
-        with self.entry_resource(IHasNoWebLink, HasNoWebLink) as resource:
+        with self.entry_resource(IHasNoWebLink, HasNoWebLink, "") as (
+            resource):
             representation = resource.toDataForJSON()
             self.assertEquals(
                 representation['self_link'], DummyAbsoluteURL.URL)
@@ -327,11 +373,10 @@
 
     testmodule_objects = [IHasOneField, HasOneField]
 
-
     def test_applyChanges_rejects_nonexistent_web_link(self):
         # If web_link is not published, applyChanges rejects a request
         # that references it.
-        with self.entry_resource(IHasOneField, HasOneField) as resource:
+        with self.entry_resource(IHasOneField, HasOneField, "") as resource:
             errors = resource.applyChanges({'web_link': u'some_value'})
             self.assertEquals(
                 errors,
@@ -341,7 +386,7 @@
         """applyChanges rejects an attempt to change web_link ."""
         self._register_website_url_space(IHasOneField)
 
-        with self.entry_resource(IHasOneField, HasOneField) as resource:
+        with self.entry_resource(IHasOneField, HasOneField, "") as resource:
             errors = resource.applyChanges({'web_link': u'some_value'})
             self.assertEquals(
                 errors,
@@ -352,7 +397,7 @@
         # value isn't actually being changed.
         self._register_website_url_space(IHasOneField)
 
-        with self.entry_resource(IHasOneField, HasOneField) as resource:
+        with self.entry_resource(IHasOneField, HasOneField, "") as resource:
             existing_web_link = resource.toDataForJSON()['web_link']
             representation = simplejson.loads(resource.applyChanges(
                 {'web_link': existing_web_link}))
@@ -372,8 +417,8 @@
         expose the right interface, it will raise an exception.
         """
         self._register_url_adapter(IHasRestrictedField)
-        with self.entry_resource(IHasRestrictedField, HasRestrictedField) as (
-            resource):
+        with self.entry_resource(
+            IHasRestrictedField, HasRestrictedField, "") as resource:
             entry = resource.entry
             entry.schema['a_field'].restrict_to_interface = IHasRestrictedField
             self.assertEquals(entry.a_field, '')
@@ -390,36 +435,83 @@
             self.assertEquals(resource.entry.a_field, 'a_value')
 
 
-class HTMLRepresentationTest(WebServiceTestCase):
+class HTMLRepresentationTest(EntryTestCase):
 
-    testmodule_objects = [HasRestrictedField, IHasRestrictedField]
+    testmodule_objects = [HasOneField, IHasOneField]
+    default_media_type = "application/xhtml+xml"
 
     def setUp(self):
         super(HTMLRepresentationTest, self).setUp()
-        getGlobalSiteManager().registerAdapter(
-            DummyAbsoluteURL, [IHasRestrictedField, IWebServiceClientRequest],
-            IAbsoluteURL)
+        self._register_url_adapter(IHasOneField)
         self.unicode_message = u"Hello from a \N{SNOWMAN}"
         self.utf8_message = self.unicode_message.encode('utf-8')
-        self.data_object = HasRestrictedField(self.unicode_message)
 
     def test_entry_html_representation_is_utf8(self):
-        request = getUtility(IWebServiceConfiguration).createRequest(
-            StringIO(""), {'HTTP_ACCEPT' : 'application/xhtml+xml'})
-        resource = EntryResource(self.data_object, request)
-        html = resource.do_GET()
-        self.assertTrue(self.utf8_message in html)
+        with self.entry_resource(
+            IHasOneField, HasOneField, self.unicode_message) as resource:
+            html = resource.do_GET()
+            self.assertTrue(self.utf8_message in html)
 
     def test_field_html_representation_is_utf8(self):
-        request = getUtility(IWebServiceConfiguration).createRequest(
-            StringIO(""), {'HTTP_ACCEPT' : 'application/xhtml+xml'})
-        entry_class = get_resource_factory(IHasRestrictedField, IEntry)
-        entry = entry_class(self.data_object, request)
-        field = entry.schema.get("a_field")
-        entry_field = EntryField(entry, field, "a_field")
-        resource = EntryFieldResource(entry_field, request)
-        html = resource.do_GET()
-        self.assertTrue(html == self.utf8_message)
+        with self.entry_field_resource(
+            IHasOneField, HasOneField, "a_field",
+            self.unicode_message) as resource:
+            html = resource.do_GET()
+            self.assertTrue(html == self.utf8_message)
+
+
+class JSONPlusHTMLRepresentationTest(EntryTestCase):
+
+    testmodule_objects = [HasTwoFields, IHasTwoFields]
+
+    def setUp(self):
+        super(JSONPlusHTMLRepresentationTest, self).setUp()
+        self.default_media_type = "application/json;include=lp_html"
+        self._register_url_adapter(IHasTwoFields)
+
+    def register_html_field_renderer(self, name=''):
+        """Simplify the register_html_field_renderer call."""
+        super(JSONPlusHTMLRepresentationTest,
+              self).register_html_field_renderer(
+            IHasTwoFields, ITextLine, simple_renderer, name)
+
+    @contextmanager
+    def resource(self, value_1="value 1", value_2 = "value 2"):
+        """Simplify the entry_resource call."""
+        with self.entry_resource(
+            IHasTwoFields, HasTwoFields, value_1, value_2) as resource:
+            yield resource
+
+    def test_normal_json_representation_omits_lp_html(self):
+        self.default_media_type = "application/json"
+        self.register_html_field_renderer()
+        with self.resource() as resource:
+            json = simplejson.loads(resource.do_GET())
+            self.assertFalse('lp_html' in json)
+
+    def test_entry_with_no_html_renderers_omits_lp_html(self):
+        with self.resource() as resource:
+            json = simplejson.loads(resource.do_GET())
+            self.assertFalse('lp_html' in json)
+
+    def test_field_specific_html_renderer_shows_up_in_lp_html(self):
+        self.register_html_field_renderer("a_field")
+        with self.resource() as resource:
+            json = simplejson.loads(resource.do_GET())
+            html = json['lp_html']
+            self.assertEquals(
+                html['a_field'], simple_renderer(resource.entry.a_field))
+
+    def test_html_renderer_for_class_renders_all_fields_of_that_class(self):
+        self.register_html_field_renderer()
+        with self.resource() as resource:
+            json = simplejson.loads(resource.do_GET())
+            html = json['lp_html']
+            self.assertEquals(
+                html['a_field'], simple_renderer(resource.entry.a_field))
+            self.assertEquals(
+                html['another_field'],
+                simple_renderer(resource.entry.another_field))
 
 
 class UnicodeChoice(EnumeratedType):
@@ -444,37 +536,30 @@
         self.a_field = value
 
 
-class UnicodeErrorTestCase(WebServiceTestCase):
+class UnicodeErrorTestCase(EntryTestCase):
     """Test that Unicode error strings are properly passed through."""
 
     testmodule_objects = [CanBeSetToUnicodeValue, ICanBeSetToUnicodeValue]
 
     def setUp(self):
         super(UnicodeErrorTestCase, self).setUp()
-        getGlobalSiteManager().registerAdapter(
-            DummyAbsoluteURL,
-            [ICanBeSetToUnicodeValue, IWebServiceClientRequest], IAbsoluteURL)
+        self._register_url_adapter(ICanBeSetToUnicodeValue)
 
     def test_unicode_error(self):
-        entry_class = get_resource_factory(ICanBeSetToUnicodeValue, IEntry)
-        request = getUtility(IWebServiceConfiguration).createRequest(
-            StringIO(""), {})
-
-        data_object = CanBeSetToUnicodeValue("")
-        entry = entry_class(data_object, request)
-        resource = EntryResource(data_object, request)
-
-        # This will raise an exception, which will cause the request
-        # to fail with a 400 error code.
-        error = resource.applyChanges({'a_field': u'No such value'})
-        self.assertEqual(request.response.getStatus(), 400)
-
-        # The error message is a Unicode string which mentions both
-        # the ASCII value and the Unicode value,
-        expected_error = (
-            u'a_field: Invalid value "No such value". Acceptable values '
-            u'are: Ascii, Uni\u00e7ode')
-        self.assertEquals(error, expected_error)
+        with self.entry_resource(
+            ICanBeSetToUnicodeValue, CanBeSetToUnicodeValue, "") as resource:
+
+            # This will raise an exception, which will cause the request
+            # to fail with a 400 error code.
+            error = resource.applyChanges({'a_field': u'No such value'})
+            self.assertEqual(resource.request.response.getStatus(), 400)
+
+            # The error message is a Unicode string which mentions both
+            # the ASCII value and the Unicode value,
+            expected_error = (
+                u'a_field: Invalid value "No such value". Acceptable values '
+                u'are: Ascii, Uni\u00e7ode')
+            self.assertEquals(error, expected_error)
 
 
 class WadlAPITestCase(WebServiceTestCase):

=== modified file 'src/lazr/restful/utils.py'
--- src/lazr/restful/utils.py	2010-09-28 12:14:12 +0000
+++ src/lazr/restful/utils.py	2011-02-17 19:35:43 +0000
@@ -9,6 +9,7 @@
     'get_current_web_service_request',
     'implement_from_dict',
     'make_identifier_safe',
+    'parse_accept_style_header',
     'safe_js_escape',
     'safe_hasattr',
     'smartquote',
@@ -55,6 +56,70 @@
     return versions.index(total_size_link_version) <= versions.index(version)
 
 
+def parse_accept_style_header(value):
+    """Parse an HTTP header from the Accept-* family.
+
+    These headers contain a list of possible values, each with an
+    optional priority.
+
+    This code is modified from Zope's
+    BrowserLanguages#getPreferredLanguages.
+
+    If a value includes parameters other than 'q', they will be preserved.
+
+    :return: All values, in descending order of priority.
+    """
+    if value is None:
+        return []
+
+    values = value.split(',')
+    values = [v.strip() for v in values if v != ""]
+
+    accepts = []
+    for index, value in enumerate(values):
+        value_and_parameters = value.split(';')
+
+        # If not supplied, quality defaults to 1.
+        quality = 1.0
+
+        if len(value_and_parameters) > 1:
+            # There's at least one parameter, possibly including
+            # a quality parameter.
+            quality_parameter = None
+            bare_value = value_and_parameters[0]
+            parameters = value_and_parameters[1:]
+            for parameter_index, parameter in enumerate(parameters):
+                if parameter.startswith('q='):
+                    # We found the quality parameter
+                    quality_parameter = parameter
+                    q = quality_parameter.split('=', 2)[1]
+                    quality = float(q)
+
+            if quality_parameter is not None:
+                # Remove the quality parameter from the list of
+                # parameters, and re-create the header value without
+                # it.
+                parameters.remove(quality_parameter)
+                if len(parameters) > 0:
+                    value = ';'.join([bare_value] + parameters)
+                else:
+                    value = bare_value
+
+        if quality == 1.0:
+            # ... but we use 1.9 - 0.001 * position to
+            # keep the ordering between all items with
+            # 1.0 quality, which may include items with no quality
+            # defined, and items with quality defined as 1.
+            quality = 1.9 - (0.001 * index)
+
+        accepts.append((quality, value))
+
+    accepts = [acc for acc in accepts if acc[0] > 0]
+    accepts.sort()
+    accepts.reverse()
+    return [value for quality, value in accepts]
+
+
 class VersionedDict(object):
     """A stack of named dictionaries.
 

=== modified file 'src/lazr/restful/version.txt'
--- src/lazr/restful/version.txt	2011-02-16 13:43:38 +0000
+++ src/lazr/restful/version.txt	2011-02-17 19:35:43 +0000
@@ -1,1 +1,1 @@
-0.16.1
+0.17.0