Merge lp:~leonardr/lazr.restful/encode-xhtml-field into lp:lazr.restful

Proposed by Leonard Richardson
Status: Merged
Approved by: Curtis Hovey
Approved revision: 166
Merged at revision: 167
Proposed branch: lp:~leonardr/lazr.restful/encode-xhtml-field
Merge into: lp:lazr.restful
Diff against target: 228 lines (+100/-11) (has conflicts)
5 files modified
src/lazr/restful/NEWS.txt (+9/-0)
src/lazr/restful/_resource.py (+15/-2)
src/lazr/restful/example/base/tests/field.txt (+26/-8)
src/lazr/restful/tests/test_webservice.py (+46/-1)
src/lazr/restful/version.txt (+4/-0)
Text conflict in src/lazr/restful/NEWS.txt
Text conflict in src/lazr/restful/tests/test_webservice.py
Text conflict in src/lazr/restful/version.txt
To merge this branch: bzr merge lp:~leonardr/lazr.restful/encode-xhtml-field
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
j.c.sackett (community) code* Approve
Review via email: mp+47536@code.launchpad.net

Description of the change

Currently, IFieldHTMLRenderer instances must return Unicode, which is encoded to UTF-8. But, the default IFieldHTMLRenderer returns UTF-8, and double-encoding that value can cause problems.

This branch normalizes the behavior of the FieldEntryResource so that IFieldHTMLRenderer instances can return *either* UTF-8 or Unicode. It makes more sense to be flexible than to force developers to remember to do the conversion themselves.

I added a unit test for the default IFieldHTMLRenderer and added to an existing pagetest to test custom IFieldHTMLRenderers.

This branch is off of revision 164 of lazr.restful and will become a point release, which will be integrated into Launchpad to unblock deployment. I did this because later revisions of lazr.restful include changes that aren't ready to go into Launchpad yet.

Here's the diff against revision 164:
http://pastebin.ubuntu.com/558575/

I'm not sure, mechanically, how to get this branch into the lazr.restful trunk. I may revert back to revision 164, commit this branch, do a release, and then re-integrate my work in progress.

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good to me. Good luck with the merge.

review: Approve (code*)
Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Leonard.

You can land this after you fix the printing of unicode characters in the doctest. That can crash the test runner (See "When to print and when to return values" in https://dev.launchpad.net/TestsStyleGuide). I recommend using .encode('ascii', 'backslashreplace') as is used in Lp answers doctests.

review: Approve (code)
167. By Leonard Richardson

Bumped version.

168. By Leonard Richardson

Got rid of bare UTF-8 characters in tests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/NEWS.txt'
2--- src/lazr/restful/NEWS.txt 2011-01-24 17:28:47 +0000
3+++ src/lazr/restful/NEWS.txt 2011-01-26 16:28:22 +0000
4@@ -2,6 +2,7 @@
5 NEWS for lazr.restful
6 =====================
7
8+<<<<<<< TREE
9 0.16.0 (Unreleased)
10 ===================
11 If each entry in the web service corresponds to some object on a
12@@ -12,6 +13,14 @@
13 You can suppress the website link for a particular entry class by
14 passing publish_web_link=False into export_as_webservice_entry().
15
16+=======
17+0.15.4 (2010-01-26)
18+===================
19+
20+Fixed inconsistent handling of custom HTML field renderings. An
21+IFieldHTMLRenderer can now return either Unicode or UTF-8.
22+
23+>>>>>>> MERGE-SOURCE
24 0.15.3 (2010-01-21)
25 ===================
26
27
28=== modified file 'src/lazr/restful/_resource.py'
29--- src/lazr/restful/_resource.py 2011-01-24 17:28:14 +0000
30+++ src/lazr/restful/_resource.py 2011-01-26 16:28:22 +0000
31@@ -151,6 +151,19 @@
32 return unicode(value)
33
34
35+def encode_value(value):
36+ """Return a UTF-8 string corresponding to `value`.
37+
38+ Non-unicode strings are assumed to be UTF-8 already.
39+ """
40+ if isinstance(value, unicode):
41+ return value.encode("utf-8")
42+ elif isinstance(value, str):
43+ return value
44+ else:
45+ return str(value)
46+
47+
48 def register_versioned_request_utility(interface, version):
49 """Registers a marker interface as a utility for version lookup.
50
51@@ -855,7 +868,7 @@
52 This is the default adapter for IFieldHTMLRenderer.
53 """
54 def unmarshall(value):
55- return cgi.escape(unicode(value).encode("utf-8"))
56+ return cgi.escape(unicode(value))
57 return unmarshall
58
59
60@@ -1280,7 +1293,7 @@
61 elif media_type == self.XHTML_TYPE:
62 name, value = self.unmarshallFieldToHTML(
63 self.context.name, self.context.field)
64- return value
65+ return encode_value(value)
66 else:
67 raise AssertionError(
68 "No representation implementation for media type %s"
69
70=== modified file 'src/lazr/restful/example/base/tests/field.txt'
71--- src/lazr/restful/example/base/tests/field.txt 2010-02-11 17:57:16 +0000
72+++ src/lazr/restful/example/base/tests/field.txt 2011-01-26 16:28:22 +0000
73@@ -277,9 +277,9 @@
74 >>> @component.adapter(ICookbook, ITextLine, IWebServiceClientRequest)
75 ... @implementer(IFieldHTMLRenderer)
76 ... def dummy_renderer(context, field, request):
77- ... """Create a simple renderer that bolds the original string."""
78+ ... """Bold the original string and add a snowman."""
79 ... def render(value):
80- ... return "<b>%s</b>" % value.encode("utf-8")
81+ ... return u"\N{SNOWMAN} <b>%s</b>" % value
82 ... return render
83
84 >>> print webservice.get(cookbook_url +'/name', 'application/xhtml+xml')
85@@ -297,10 +297,16 @@
86 ...and the XHTML representation of an ICookbook's description will be the
87 result of calling a dummy_renderer object.
88
89- >>> print webservice.get(field_url, 'application/xhtml+xml')
90+ >>> def encode_response(response):
91+ ... """Encode a response that contains weird Unicode characters."""
92+ ... response_unicode = str(response).decode("utf-8")
93+ ... return response_unicode.encode("ascii", "backslashreplace")
94+
95+ >>> response = webservice.get(field_url, 'application/xhtml+xml')
96+ >>> print encode_response(response)
97 HTTP/1.1 200 Ok
98 ...
99- <b>Description</b>
100+ \u2603 <b>Description</b>
101
102 In fact, that adapter will be used for every ITextLine field of an
103 ICookbook.
104@@ -357,12 +363,23 @@
105 creates a custom renderer for ICookbook.description, by registering
106 a view on ICookbook called "description".
107
108+ >>> @component.adapter(ICookbook, ITextLine, IWebServiceClientRequest)
109+ ... @implementer(IFieldHTMLRenderer)
110+ ... def dummy_renderer(context, field, request):
111+ ... """Bold the original string, add a snowman, and encode UTF-8."""
112+ ... def render(value):
113+ ... return (u"\N{SNOWMAN} <b>%s</b>" % value).encode("utf-8")
114+ ... return render
115 >>> manager.registerAdapter(dummy_renderer, name='description')
116
117- >>> print webservice.get(field_url, 'application/xhtml+xml')
118+This renderer is identical to the one shown earlier, except that it
119+returns UTF-8 instead of Unicode.
120+
121+ >>> response = webservice.get(field_url, 'application/xhtml+xml')
122+ >>> print encode_response(response)
123 HTTP/1.1 200 Ok
124 ...
125- <b>Description</b>
126+ \u2603 <b>Description</b>
127
128 Unlike what happened when we registered a renderer for
129 ICookbook/ITextLine, other ITextLine fields of ICookbook are not
130@@ -376,11 +393,12 @@
131 The XHTML representation of an entry incorporates any custom XHTML
132 representations of that entry's fields.
133
134- >>> print webservice.get(cookbook_url, 'application/xhtml+xml')
135+ >>> response = webservice.get(cookbook_url, 'application/xhtml+xml')
136+ >>> print encode_response(response)
137 HTTP/1.1 200 Ok
138 ...
139 <dt>description</dt>
140- <dd><b>Description</b></dd>
141+ <dd>\u2603 <b>Description</b></dd>
142 ...
143
144 Before we continue, here's some code to unregister the view.
145
146=== modified file 'src/lazr/restful/tests/test_webservice.py'
147--- src/lazr/restful/tests/test_webservice.py 2011-01-24 22:00:56 +0000
148+++ src/lazr/restful/tests/test_webservice.py 2011-01-26 16:28:22 +0000
149@@ -25,9 +25,13 @@
150 from zope.traversing.browser.interfaces import IAbsoluteURL
151
152 from lazr.enum import EnumeratedType, Item
153-from lazr.restful import ResourceOperation
154+from lazr.restful import (
155+ EntryField,
156+ ResourceOperation,
157+ )
158 from lazr.restful.fields import Reference
159 from lazr.restful.interfaces import (
160+<<<<<<< TREE
161 ICollection,
162 IEntry,
163 IResourceGETOperation,
164@@ -38,6 +42,15 @@
165 IWebServiceVersion,
166 )
167 from lazr.restful import EntryResource, ResourceGETOperation
168+=======
169+ ICollection, IEntry, IResourceGETOperation, IServiceRootResource,
170+ IWebServiceConfiguration, IWebServiceClientRequest, IWebServiceVersion)
171+from lazr.restful import (
172+ EntryFieldResource,
173+ EntryResource,
174+ ResourceGETOperation,
175+ )
176+>>>>>>> MERGE-SOURCE
177 from lazr.restful.declarations import (
178 exported, export_as_webservice_entry, LAZR_WEBSERVICE_NAME, annotate_exported_methods)
179 from lazr.restful.testing.webservice import (
180@@ -314,6 +327,38 @@
181 self.assertEquals(resource.entry.a_field, 'a_value')
182
183
184+class HTMLRepresentationTest(WebServiceTestCase):
185+
186+ testmodule_objects = [HasRestrictedField, IHasRestrictedField]
187+
188+ def setUp(self):
189+ super(HTMLRepresentationTest, self).setUp()
190+ getGlobalSiteManager().registerAdapter(
191+ DummyAbsoluteURL, [IHasRestrictedField, IWebServiceClientRequest],
192+ IAbsoluteURL)
193+ self.unicode_message = u"Hello from a \N{SNOWMAN}"
194+ self.utf8_message = self.unicode_message.encode('utf-8')
195+ self.data_object = HasRestrictedField(self.unicode_message)
196+
197+ def test_entry_html_representation_is_utf8(self):
198+ request = getUtility(IWebServiceConfiguration).createRequest(
199+ StringIO(""), {'HTTP_ACCEPT' : 'application/xhtml+xml'})
200+ resource = EntryResource(self.data_object, request)
201+ html = resource.do_GET()
202+ self.assertTrue(self.utf8_message in html)
203+
204+ def test_field_html_representation_is_utf8(self):
205+ request = getUtility(IWebServiceConfiguration).createRequest(
206+ StringIO(""), {'HTTP_ACCEPT' : 'application/xhtml+xml'})
207+ entry_class = get_resource_factory(IHasRestrictedField, IEntry)
208+ entry = entry_class(self.data_object, request)
209+ field = entry.schema.get("a_field")
210+ entry_field = EntryField(entry, field, "a_field")
211+ resource = EntryFieldResource(entry_field, request)
212+ html = resource.do_GET()
213+ self.assertTrue(html == self.utf8_message)
214+
215+
216 class UnicodeChoice(EnumeratedType):
217 """A choice between an ASCII value and a Unicode value."""
218 ASCII = Item("Ascii", "Ascii choice")
219
220=== modified file 'src/lazr/restful/version.txt'
221--- src/lazr/restful/version.txt 2011-01-21 21:12:17 +0000
222+++ src/lazr/restful/version.txt 2011-01-26 16:28:22 +0000
223@@ -1,1 +1,5 @@
224+<<<<<<< TREE
225 0.16.0
226+=======
227+0.15.4
228+>>>>>>> MERGE-SOURCE

Subscribers

People subscribed via source and target branches