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
=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py 2012-01-19 22:59:03 +0000
+++ lib/lp/registry/browser/__init__.py 2012-01-27 19:21:54 +0000
@@ -37,6 +37,7 @@
37 )37 )
38from lp.services.webapp.publisher import (38from lp.services.webapp.publisher import (
39 canonical_url,39 canonical_url,
40 DataDownloadView,
40 LaunchpadView,41 LaunchpadView,
41 )42 )
4243
@@ -271,17 +272,15 @@
271 self.updateContextFromData(data)272 self.updateContextFromData(data)
272273
273274
274class BaseRdfView:275class BaseRdfView(DataDownloadView):
275 """A view that sets its mime-type to application/rdf+xml."""276 """A view that sets its mime-type to application/rdf+xml."""
276277
277 template = None278 template = None
278 filename = None279 filename = None
279280
280 def __init__(self, context, request):281 content_type = 'application/rdf+xml'
281 self.context = context
282 self.request = request
283282
284 def __call__(self):283 def getBody(self):
285 """Render RDF output, and return it as a string encoded in UTF-8.284 """Render RDF output, and return it as a string encoded in UTF-8.
286285
287 Render the page template to produce RDF output.286 Render the page template to produce RDF output.
@@ -289,10 +288,6 @@
289288
290 As a side-effect, HTTP headers are set for the mime type289 As a side-effect, HTTP headers are set for the mime type
291 and filename for download."""290 and filename for download."""
292 self.request.response.setHeader('Content-Type', 'application/rdf+xml')
293 self.request.response.setHeader(
294 'Content-Disposition', 'attachment; filename=%s.rdf' % (
295 self.filename))
296 unicodedata = self.template()291 unicodedata = self.template()
297 encodeddata = unicodedata.encode('utf-8')292 encodeddata = unicodedata.encode('utf-8')
298 return encodeddata293 return encodeddata
299294
=== modified file 'lib/lp/registry/browser/codeofconduct.py'
--- lib/lp/registry/browser/codeofconduct.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/codeofconduct.py 2012-01-27 19:21:54 +0000
@@ -45,6 +45,7 @@
45 Link,45 Link,
46 )46 )
47from lp.services.webapp.interfaces import ILaunchBag47from lp.services.webapp.interfaces import ILaunchBag
48from lp.services.webapp.publisher import DataDownloadView
4849
4950
50class SignedCodeOfConductSetNavigation(GetitemNavigation):51class SignedCodeOfConductSetNavigation(GetitemNavigation):
@@ -132,34 +133,25 @@
132 return self.context.title133 return self.context.title
133134
134135
135class CodeOfConductDownloadView:136class CodeOfConductDownloadView(DataDownloadView):
136 """Download view class for CoC page.137 """Download view class for CoC page.
137138
138 This view does not use a template, but uses a __call__ method139 This view provides a text file with "Content-disposition: attachment",
139 that returns a file to the browser.140 causing browsers to download rather than display it.
140 """141 """
141142
142 def __init__(self, context, request):143 content_type = 'text/plain'
143 self.context = context
144 self.request = request
145144
146 def __call__(self):145 def getBody(self):
147 """Set response headers to download an attachment, and return
148 CoC file data.
149 """
150 # Use the context attribute 'content' as data to return.146 # Use the context attribute 'content' as data to return.
151 # Avoid open the CoC file again.147 # Avoid open the CoC file again.
152 content = self.context.content148 return self.context.content
153149
150 @property
151 def filename(self):
154 # Build a fancy filename:152 # Build a fancy filename:
155 # - Use title with no spaces and append '.txt'153 # - Use title with no spaces and append '.txt'
156 filename = self.context.title.replace(' ', '') + '.txt'154 return self.context.title.replace(' ', '') + '.txt'
157
158 self.request.response.setHeader('Content-Type', 'application/text')
159 self.request.response.setHeader('Content-Length', len(content))
160 self.request.response.setHeader(
161 'Content-Disposition', 'attachment; filename="%s"' % filename)
162 return content
163155
164156
165class CodeOfConductSetView(LaunchpadView):157class CodeOfConductSetView(LaunchpadView):
166158
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2012-01-25 04:56:50 +0000
+++ lib/lp/registry/browser/person.py 2012-01-27 19:21:54 +0000
@@ -1195,7 +1195,7 @@
11951195
1196 @property1196 @property
1197 def filename(self):1197 def filename(self):
1198 return self.context.name1198 return '%s.rdf' % self.context.name
11991199
12001200
1201class PersonRdfContentsView:1201class PersonRdfContentsView:
12021202
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/product.py 2012-01-27 19:21:54 +0000
@@ -1814,7 +1814,7 @@
18141814
1815 @property1815 @property
1816 def filename(self):1816 def filename(self):
1817 return self.context.name1817 return '%s.rdf' % self.context.name
18181818
18191819
1820class Icon:1820class Icon:
18211821
=== modified file 'lib/lp/registry/browser/productrelease.py'
--- lib/lp/registry/browser/productrelease.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/productrelease.py 2012-01-27 19:21:54 +0000
@@ -263,7 +263,7 @@
263263
264 @property264 @property
265 def filename(self):265 def filename(self):
266 return '%s-%s-%s' % (266 return '%s-%s-%s.rdf' % (
267 self.context.product.name,267 self.context.product.name,
268 self.context.productseries.name,268 self.context.productseries.name,
269 self.context.version)269 self.context.version)
270270
=== modified file 'lib/lp/registry/browser/productseries.py'
--- lib/lp/registry/browser/productseries.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/productseries.py 2012-01-27 19:21:54 +0000
@@ -1240,7 +1240,7 @@
12401240
1241 @property1241 @property
1242 def filename(self):1242 def filename(self):
1243 return '%s-%s' % (self.context.product.name, self.context.name)1243 return '%s-%s.rdf' % (self.context.product.name, self.context.name)
12441244
12451245
1246class ProductSeriesFileBugRedirect(LaunchpadView):1246class ProductSeriesFileBugRedirect(LaunchpadView):
12471247
=== modified file 'lib/lp/registry/browser/tests/test_codeofconduct.py'
--- lib/lp/registry/browser/tests/test_codeofconduct.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_codeofconduct.py 2012-01-27 19:21:54 +0000
@@ -7,9 +7,13 @@
77
8from zope.component import getUtility8from zope.component import getUtility
99
10from lp.registry.interfaces.codeofconduct import ISignedCodeOfConductSet10from lp.registry.interfaces.codeofconduct import (
11 ICodeOfConductSet,
12 ISignedCodeOfConductSet,
13 )
11from lp.registry.model.codeofconduct import SignedCodeOfConduct14from lp.registry.model.codeofconduct import SignedCodeOfConduct
12from lp.testing import (15from lp.testing import (
16 BrowserTestCase,
13 login_celebrity,17 login_celebrity,
14 TestCaseWithFactory,18 TestCaseWithFactory,
15 )19 )
@@ -147,3 +151,20 @@
147151
148 def test_admincomment_required(self):152 def test_admincomment_required(self):
149 self.verify_admincomment_required('Deactivate', '+deactivate')153 self.verify_admincomment_required('Deactivate', '+deactivate')
154
155
156class TestCodeOfConductBrowser(BrowserTestCase):
157 """Test the download view for the CoC."""
158
159 layer = DatabaseFunctionalLayer
160
161 def test_response(self):
162 """Ensure the headers and body are as expected."""
163 coc = getUtility(ICodeOfConductSet)['1.1']
164 content = coc.content
165 browser = self.getViewBrowser(coc, '+download')
166 self.assertEqual(content, browser.contents)
167 self.assertEqual(str(len(content)), browser.headers['Content-length'])
168 disposition = 'attachment; filename="UbuntuCodeofConduct-1.1.txt"'
169 self.assertEqual(disposition, browser.headers['Content-disposition'])
170 self.assertEqual('text/plain', browser.headers['Content-type'])
150171
=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_person.py 2012-01-27 19:21:54 +0000
@@ -11,6 +11,7 @@
11from lp.services.config import config11from lp.services.config import config
12from lp.services.webapp.servers import LaunchpadTestRequest12from lp.services.webapp.servers import LaunchpadTestRequest
13from lp.testing import (13from lp.testing import (
14 BrowserTestCase,
14 login_person,15 login_person,
15 TestCaseWithFactory,16 TestCaseWithFactory,
16 )17 )
@@ -50,3 +51,19 @@
50 '''))51 '''))
51 self.assertEquals(52 self.assertEquals(
52 'http://prod.launchpad.dev/~eris', self.view.openid_identity_url)53 'http://prod.launchpad.dev/~eris', self.view.openid_identity_url)
54
55
56class TestPersonRdfView(BrowserTestCase):
57 """Test the RDF view."""
58
59 layer = DatabaseFunctionalLayer
60
61 def test_headers(self):
62 """The headers for the RDF view of a person should be as expected."""
63 person = self.factory.makePerson()
64 content_disposition = 'attachment; filename="%s.rdf"' % person.name
65 browser = self.getViewBrowser(person, view_name='+rdf')
66 self.assertEqual(
67 content_disposition, browser.headers['Content-disposition'])
68 self.assertEqual(
69 'application/rdf+xml', browser.headers['Content-type'])
5370
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2012-01-27 19:21:54 +0000
@@ -20,6 +20,7 @@
20from lp.services.config import config20from lp.services.config import config
21from lp.services.webapp.publisher import canonical_url21from lp.services.webapp.publisher import canonical_url
22from lp.testing import (22from lp.testing import (
23 BrowserTestCase,
23 login_celebrity,24 login_celebrity,
24 login_person,25 login_person,
25 person_logged_in,26 person_logged_in,
@@ -386,3 +387,19 @@
386 self.assertTrue(387 self.assertTrue(
387 'Y.lp.app.choice.addBinaryChoice' in str(388 'Y.lp.app.choice.addBinaryChoice' in str(
388 content.find(id='fnord-edit-license-approved').parent))389 content.find(id='fnord-edit-license-approved').parent))
390
391
392class TestProductRdfView(BrowserTestCase):
393 """Test the Product RDF view."""
394
395 layer = DatabaseFunctionalLayer
396
397 def test_headers(self):
398 """The headers for the RDF view of a product should be as expected."""
399 product = self.factory.makeProduct()
400 browser = self.getViewBrowser(product, view_name='+rdf')
401 content_disposition = 'attachment; filename="%s.rdf"' % product.name
402 self.assertEqual(
403 content_disposition, browser.headers['Content-disposition'])
404 self.assertEqual(
405 'application/rdf+xml', browser.headers['Content-type'])
389406
=== modified file 'lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt'
--- lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/registry/stories/productrelease/xx-productrelease-rdf.txt 2012-01-27 19:21:54 +0000
@@ -1,13 +1,13 @@
1Check that the productrelease RDF export works.1Check that the productrelease RDF export works.
22
3 >>> print http(r"""3 >>> print http(r"""
4 ... GET /firefox/trunk/0.9/+rdf HTTP/1.14 ... GET /firefox/trunk/0.9/+rdf HTTP/1.1
5 ... """)5 ... """)
6 HTTP/1.1 200 Ok6 HTTP/1.1 200 Ok
7 Content-Disposition: attachment; filename=firefox-trunk-0.9.rdf7 Content-Disposition: attachment; filename="firefox-trunk-0.9.rdf"
8 Content-Length: ...8 Content-Length: ...
9 Content-Type: application/rdf+xml9 Content-Type: application/rdf+xml
10 Vary: ...10 Vary: ...
11 <BLANKLINE>11 <BLANKLINE>
12 <?xml version="1.0" encoding="utf-8"...?>12 <?xml version="1.0" encoding="utf-8"...?>
13 <rdf:RDF ...13 <rdf:RDF ...
1414
=== modified file 'lib/lp/registry/stories/productseries/xx-productseries-rdf.txt'
--- lib/lp/registry/stories/productseries/xx-productseries-rdf.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/registry/stories/productseries/xx-productseries-rdf.txt 2012-01-27 19:21:54 +0000
@@ -1,13 +1,13 @@
1Check that the productseries RDF export works.1Check that the productseries RDF export works.
22
3 >>> print http(r"""3 >>> print http(r"""
4 ... GET /firefox/trunk/+rdf HTTP/1.14 ... GET /firefox/trunk/+rdf HTTP/1.1
5 ... """)5 ... """)
6 HTTP/1.1 200 Ok6 HTTP/1.1 200 Ok
7 Content-Disposition: attachment; filename=firefox-trunk.rdf7 Content-Disposition: attachment; filename="firefox-trunk.rdf"
8 Content-Length: ...8 Content-Length: ...
9 Content-Type: application/rdf+xml9 Content-Type: application/rdf+xml
10 Vary: ...10 Vary: ...
11 <BLANKLINE>11 <BLANKLINE>
12 <?xml version="1.0" encoding="utf-8"...?>12 <?xml version="1.0" encoding="utf-8"...?>
13 <rdf:RDF ...13 <rdf:RDF ...
1414
=== modified file 'lib/lp/services/webapp/publisher.py'
--- lib/lp/services/webapp/publisher.py 2012-01-23 04:05:17 +0000
+++ lib/lp/services/webapp/publisher.py 2012-01-27 19:21:54 +0000
@@ -7,6 +7,7 @@
77
8__metaclass__ = type8__metaclass__ = type
9__all__ = [9__all__ = [
10 'DataDownloadView',
10 'LaunchpadContainer',11 'LaunchpadContainer',
11 'LaunchpadView',12 'LaunchpadView',
12 'LaunchpadXMLRPCView',13 'LaunchpadXMLRPCView',
@@ -220,6 +221,29 @@
220 return cls221 return cls
221222
222223
224class DataDownloadView:
225 """Download data without templating.
226
227 Subclasses must provide getBody, content_type and filename.
228 """
229
230 def __init__(self, context, request):
231 self.context = context
232 self.request = request
233
234 def __call__(self):
235 """Set the headers and return the body.
236
237 It is not necessary to supply Content-length, because this is added by
238 the caller.
239 """
240 self.request.response.setHeader('Content-Type', self.content_type)
241 self.request.response.setHeader(
242 'Content-Disposition', 'attachment; filename="%s"' % (
243 self.filename))
244 return self.getBody()
245
246
223class UserAttributeCache:247class UserAttributeCache:
224 """Mix in to provide self.user, cached."""248 """Mix in to provide self.user, cached."""
225249