Merge lp:~abentley/launchpad/data-download-view into lp:launchpad

Proposed by Aaron Bentley on 2012-01-26
Status: Merged
Approved by: j.c.sackett on 2012-01-26
Approved revision: no longer in the source branch.
Merged at revision: 14739
Proposed branch: lp:~abentley/launchpad/data-download-view
Merge into: lp:launchpad
Prerequisite: lp:~abentley/launchpad/bugcomment-as-icomment
Diff against target: 354 lines (+120/-54)
12 files modified
lib/lp/registry/browser/__init__.py (+4/-9)
lib/lp/registry/browser/codeofconduct.py (+10/-18)
lib/lp/registry/browser/person.py (+1/-1)
lib/lp/registry/browser/product.py (+1/-1)
lib/lp/registry/browser/productrelease.py (+1/-1)
lib/lp/registry/browser/productseries.py (+1/-1)
lib/lp/registry/browser/tests/test_codeofconduct.py (+22/-1)
lib/lp/registry/browser/tests/test_person.py (+17/-0)
lib/lp/registry/browser/tests/test_product.py (+17/-0)
lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt (+11/-11)
lib/lp/registry/stories/productseries/xx-productseries-rdf.txt (+11/-11)
lib/lp/services/webapp/publisher.py (+24/-0)
To merge this branch: bzr merge lp:~abentley/launchpad/data-download-view
Reviewer Review Type Date Requested Status
j.c.sackett (community) 2012-01-26 Approve on 2012-01-26
Richard Harding (community) code* 2012-01-26 Approve on 2012-01-26
Review via email: mp+90269@code.launchpad.net

Commit Message

Extract DataDownloadView

Description of the Change

= Summary =
Extract DataDownloadView from CodeOfConductDownloadView and BaseRdfView

== Proposed fix ==
See above

== Pre-implementation notes ==
Discussed Content-type change with sinzui.

== Implementation details ==
Changed Content-Type of downloaded CoC from the fictitious 'application/text' to the standard 'text/plain'. We believe this was a hack to ensure users were prompted to save the CoC, but the Content-disposition of "attachment" achieves this well.

The filename parameter of the Content-disposition header is now always quoted, in compliance with the RFC.

Changed the indentation of a couple of test cases, to fix lint errors.

Subclasses of BaseRdfView must now specify the extension, as well as the rest
of the filename.

== Tests ==
bin/test -t test_codeofconduct -t test_person -t test_product -t xx-productrelease-rdf.txt -t xx-productseries-rdf.txt

== Demo and Q/A ==
Visit https://qastaging.launchpad.net/codeofconduct/1.1/+download and you should be prompted to save the file. The text should be the Code of Conduct.

Visit https://qastaging.launchpad.net/launchpad/+rdf and you should be prompted to save the file. The text should be RDF.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/__init__.py
  lib/lp/registry/browser/codeofconduct.py
  lib/lp/registry/browser/person.py
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/productrelease.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/test_codeofconduct.py
  lib/lp/registry/browser/tests/test_person.py
  lib/lp/registry/browser/tests/test_product.py
  lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt
  lib/lp/registry/stories/productseries/xx-productseries-rdf.txt
  lib/lp/services/webapp/publisher.py

To post a comment you must log in.
Richard Harding (rharding) :
review: Approve (code*)
j.c.sackett (jcsackett) wrote :

I have nothing to add. This is a very solid change, thanks Aaron.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/__init__.py'
2--- lib/lp/registry/browser/__init__.py 2012-01-19 22:59:03 +0000
3+++ lib/lp/registry/browser/__init__.py 2012-01-27 19:21:54 +0000
4@@ -37,6 +37,7 @@
5 )
6 from lp.services.webapp.publisher import (
7 canonical_url,
8+ DataDownloadView,
9 LaunchpadView,
10 )
11
12@@ -271,17 +272,15 @@
13 self.updateContextFromData(data)
14
15
16-class BaseRdfView:
17+class BaseRdfView(DataDownloadView):
18 """A view that sets its mime-type to application/rdf+xml."""
19
20 template = None
21 filename = None
22
23- def __init__(self, context, request):
24- self.context = context
25- self.request = request
26+ content_type = 'application/rdf+xml'
27
28- def __call__(self):
29+ def getBody(self):
30 """Render RDF output, and return it as a string encoded in UTF-8.
31
32 Render the page template to produce RDF output.
33@@ -289,10 +288,6 @@
34
35 As a side-effect, HTTP headers are set for the mime type
36 and filename for download."""
37- self.request.response.setHeader('Content-Type', 'application/rdf+xml')
38- self.request.response.setHeader(
39- 'Content-Disposition', 'attachment; filename=%s.rdf' % (
40- self.filename))
41 unicodedata = self.template()
42 encodeddata = unicodedata.encode('utf-8')
43 return encodeddata
44
45=== modified file 'lib/lp/registry/browser/codeofconduct.py'
46--- lib/lp/registry/browser/codeofconduct.py 2012-01-01 02:58:52 +0000
47+++ lib/lp/registry/browser/codeofconduct.py 2012-01-27 19:21:54 +0000
48@@ -45,6 +45,7 @@
49 Link,
50 )
51 from lp.services.webapp.interfaces import ILaunchBag
52+from lp.services.webapp.publisher import DataDownloadView
53
54
55 class SignedCodeOfConductSetNavigation(GetitemNavigation):
56@@ -132,34 +133,25 @@
57 return self.context.title
58
59
60-class CodeOfConductDownloadView:
61+class CodeOfConductDownloadView(DataDownloadView):
62 """Download view class for CoC page.
63
64- This view does not use a template, but uses a __call__ method
65- that returns a file to the browser.
66+ This view provides a text file with "Content-disposition: attachment",
67+ causing browsers to download rather than display it.
68 """
69
70- def __init__(self, context, request):
71- self.context = context
72- self.request = request
73+ content_type = 'text/plain'
74
75- def __call__(self):
76- """Set response headers to download an attachment, and return
77- CoC file data.
78- """
79+ def getBody(self):
80 # Use the context attribute 'content' as data to return.
81 # Avoid open the CoC file again.
82- content = self.context.content
83+ return self.context.content
84
85+ @property
86+ def filename(self):
87 # Build a fancy filename:
88 # - Use title with no spaces and append '.txt'
89- filename = self.context.title.replace(' ', '') + '.txt'
90-
91- self.request.response.setHeader('Content-Type', 'application/text')
92- self.request.response.setHeader('Content-Length', len(content))
93- self.request.response.setHeader(
94- 'Content-Disposition', 'attachment; filename="%s"' % filename)
95- return content
96+ return self.context.title.replace(' ', '') + '.txt'
97
98
99 class CodeOfConductSetView(LaunchpadView):
100
101=== modified file 'lib/lp/registry/browser/person.py'
102--- lib/lp/registry/browser/person.py 2012-01-25 04:56:50 +0000
103+++ lib/lp/registry/browser/person.py 2012-01-27 19:21:54 +0000
104@@ -1195,7 +1195,7 @@
105
106 @property
107 def filename(self):
108- return self.context.name
109+ return '%s.rdf' % self.context.name
110
111
112 class PersonRdfContentsView:
113
114=== modified file 'lib/lp/registry/browser/product.py'
115--- lib/lp/registry/browser/product.py 2012-01-01 02:58:52 +0000
116+++ lib/lp/registry/browser/product.py 2012-01-27 19:21:54 +0000
117@@ -1814,7 +1814,7 @@
118
119 @property
120 def filename(self):
121- return self.context.name
122+ return '%s.rdf' % self.context.name
123
124
125 class Icon:
126
127=== modified file 'lib/lp/registry/browser/productrelease.py'
128--- lib/lp/registry/browser/productrelease.py 2012-01-01 02:58:52 +0000
129+++ lib/lp/registry/browser/productrelease.py 2012-01-27 19:21:54 +0000
130@@ -263,7 +263,7 @@
131
132 @property
133 def filename(self):
134- return '%s-%s-%s' % (
135+ return '%s-%s-%s.rdf' % (
136 self.context.product.name,
137 self.context.productseries.name,
138 self.context.version)
139
140=== modified file 'lib/lp/registry/browser/productseries.py'
141--- lib/lp/registry/browser/productseries.py 2012-01-01 02:58:52 +0000
142+++ lib/lp/registry/browser/productseries.py 2012-01-27 19:21:54 +0000
143@@ -1240,7 +1240,7 @@
144
145 @property
146 def filename(self):
147- return '%s-%s' % (self.context.product.name, self.context.name)
148+ return '%s-%s.rdf' % (self.context.product.name, self.context.name)
149
150
151 class ProductSeriesFileBugRedirect(LaunchpadView):
152
153=== modified file 'lib/lp/registry/browser/tests/test_codeofconduct.py'
154--- lib/lp/registry/browser/tests/test_codeofconduct.py 2012-01-01 02:58:52 +0000
155+++ lib/lp/registry/browser/tests/test_codeofconduct.py 2012-01-27 19:21:54 +0000
156@@ -7,9 +7,13 @@
157
158 from zope.component import getUtility
159
160-from lp.registry.interfaces.codeofconduct import ISignedCodeOfConductSet
161+from lp.registry.interfaces.codeofconduct import (
162+ ICodeOfConductSet,
163+ ISignedCodeOfConductSet,
164+ )
165 from lp.registry.model.codeofconduct import SignedCodeOfConduct
166 from lp.testing import (
167+ BrowserTestCase,
168 login_celebrity,
169 TestCaseWithFactory,
170 )
171@@ -147,3 +151,20 @@
172
173 def test_admincomment_required(self):
174 self.verify_admincomment_required('Deactivate', '+deactivate')
175+
176+
177+class TestCodeOfConductBrowser(BrowserTestCase):
178+ """Test the download view for the CoC."""
179+
180+ layer = DatabaseFunctionalLayer
181+
182+ def test_response(self):
183+ """Ensure the headers and body are as expected."""
184+ coc = getUtility(ICodeOfConductSet)['1.1']
185+ content = coc.content
186+ browser = self.getViewBrowser(coc, '+download')
187+ self.assertEqual(content, browser.contents)
188+ self.assertEqual(str(len(content)), browser.headers['Content-length'])
189+ disposition = 'attachment; filename="UbuntuCodeofConduct-1.1.txt"'
190+ self.assertEqual(disposition, browser.headers['Content-disposition'])
191+ self.assertEqual('text/plain', browser.headers['Content-type'])
192
193=== modified file 'lib/lp/registry/browser/tests/test_person.py'
194--- lib/lp/registry/browser/tests/test_person.py 2012-01-01 02:58:52 +0000
195+++ lib/lp/registry/browser/tests/test_person.py 2012-01-27 19:21:54 +0000
196@@ -11,6 +11,7 @@
197 from lp.services.config import config
198 from lp.services.webapp.servers import LaunchpadTestRequest
199 from lp.testing import (
200+ BrowserTestCase,
201 login_person,
202 TestCaseWithFactory,
203 )
204@@ -50,3 +51,19 @@
205 '''))
206 self.assertEquals(
207 'http://prod.launchpad.dev/~eris', self.view.openid_identity_url)
208+
209+
210+class TestPersonRdfView(BrowserTestCase):
211+ """Test the RDF view."""
212+
213+ layer = DatabaseFunctionalLayer
214+
215+ def test_headers(self):
216+ """The headers for the RDF view of a person should be as expected."""
217+ person = self.factory.makePerson()
218+ content_disposition = 'attachment; filename="%s.rdf"' % person.name
219+ browser = self.getViewBrowser(person, view_name='+rdf')
220+ self.assertEqual(
221+ content_disposition, browser.headers['Content-disposition'])
222+ self.assertEqual(
223+ 'application/rdf+xml', browser.headers['Content-type'])
224
225=== modified file 'lib/lp/registry/browser/tests/test_product.py'
226--- lib/lp/registry/browser/tests/test_product.py 2012-01-01 02:58:52 +0000
227+++ lib/lp/registry/browser/tests/test_product.py 2012-01-27 19:21:54 +0000
228@@ -20,6 +20,7 @@
229 from lp.services.config import config
230 from lp.services.webapp.publisher import canonical_url
231 from lp.testing import (
232+ BrowserTestCase,
233 login_celebrity,
234 login_person,
235 person_logged_in,
236@@ -386,3 +387,19 @@
237 self.assertTrue(
238 'Y.lp.app.choice.addBinaryChoice' in str(
239 content.find(id='fnord-edit-license-approved').parent))
240+
241+
242+class TestProductRdfView(BrowserTestCase):
243+ """Test the Product RDF view."""
244+
245+ layer = DatabaseFunctionalLayer
246+
247+ def test_headers(self):
248+ """The headers for the RDF view of a product should be as expected."""
249+ product = self.factory.makeProduct()
250+ browser = self.getViewBrowser(product, view_name='+rdf')
251+ content_disposition = 'attachment; filename="%s.rdf"' % product.name
252+ self.assertEqual(
253+ content_disposition, browser.headers['Content-disposition'])
254+ self.assertEqual(
255+ 'application/rdf+xml', browser.headers['Content-type'])
256
257=== modified file 'lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt'
258--- lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt 2009-06-12 16:36:02 +0000
259+++ lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt 2012-01-27 19:21:54 +0000
260@@ -1,13 +1,13 @@
261 Check that the productrelease RDF export works.
262
263- >>> print http(r"""
264- ... GET /firefox/trunk/0.9/+rdf HTTP/1.1
265- ... """)
266- HTTP/1.1 200 Ok
267- Content-Disposition: attachment; filename=firefox-trunk-0.9.rdf
268- Content-Length: ...
269- Content-Type: application/rdf+xml
270- Vary: ...
271- <BLANKLINE>
272- <?xml version="1.0" encoding="utf-8"...?>
273- <rdf:RDF ...
274+ >>> print http(r"""
275+ ... GET /firefox/trunk/0.9/+rdf HTTP/1.1
276+ ... """)
277+ HTTP/1.1 200 Ok
278+ Content-Disposition: attachment; filename="firefox-trunk-0.9.rdf"
279+ Content-Length: ...
280+ Content-Type: application/rdf+xml
281+ Vary: ...
282+ <BLANKLINE>
283+ <?xml version="1.0" encoding="utf-8"...?>
284+ <rdf:RDF ...
285
286=== modified file 'lib/lp/registry/stories/productseries/xx-productseries-rdf.txt'
287--- lib/lp/registry/stories/productseries/xx-productseries-rdf.txt 2009-06-12 16:36:02 +0000
288+++ lib/lp/registry/stories/productseries/xx-productseries-rdf.txt 2012-01-27 19:21:54 +0000
289@@ -1,13 +1,13 @@
290 Check that the productseries RDF export works.
291
292- >>> print http(r"""
293- ... GET /firefox/trunk/+rdf HTTP/1.1
294- ... """)
295- HTTP/1.1 200 Ok
296- Content-Disposition: attachment; filename=firefox-trunk.rdf
297- Content-Length: ...
298- Content-Type: application/rdf+xml
299- Vary: ...
300- <BLANKLINE>
301- <?xml version="1.0" encoding="utf-8"...?>
302- <rdf:RDF ...
303+ >>> print http(r"""
304+ ... GET /firefox/trunk/+rdf HTTP/1.1
305+ ... """)
306+ HTTP/1.1 200 Ok
307+ Content-Disposition: attachment; filename="firefox-trunk.rdf"
308+ Content-Length: ...
309+ Content-Type: application/rdf+xml
310+ Vary: ...
311+ <BLANKLINE>
312+ <?xml version="1.0" encoding="utf-8"...?>
313+ <rdf:RDF ...
314
315=== modified file 'lib/lp/services/webapp/publisher.py'
316--- lib/lp/services/webapp/publisher.py 2012-01-23 04:05:17 +0000
317+++ lib/lp/services/webapp/publisher.py 2012-01-27 19:21:54 +0000
318@@ -7,6 +7,7 @@
319
320 __metaclass__ = type
321 __all__ = [
322+ 'DataDownloadView',
323 'LaunchpadContainer',
324 'LaunchpadView',
325 'LaunchpadXMLRPCView',
326@@ -220,6 +221,29 @@
327 return cls
328
329
330+class DataDownloadView:
331+ """Download data without templating.
332+
333+ Subclasses must provide getBody, content_type and filename.
334+ """
335+
336+ def __init__(self, context, request):
337+ self.context = context
338+ self.request = request
339+
340+ def __call__(self):
341+ """Set the headers and return the body.
342+
343+ It is not necessary to supply Content-length, because this is added by
344+ the caller.
345+ """
346+ self.request.response.setHeader('Content-Type', self.content_type)
347+ self.request.response.setHeader(
348+ 'Content-Disposition', 'attachment; filename="%s"' % (
349+ self.filename))
350+ return self.getBody()
351+
352+
353 class UserAttributeCache:
354 """Mix in to provide self.user, cached."""
355