launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06185
[Merge] lp:~abentley/launchpad/data-download-view into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/data-download-view into lp:launchpad with lp:~abentley/launchpad/bugcomment-as-icomment as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~abentley/launchpad/data-download-view/+merge/90269
= 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
--
https://code.launchpad.net/~abentley/launchpad/data-download-view/+merge/90269
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/data-download-view into lp:launchpad.
=== 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-26 14:51:36 +0000
@@ -37,6 +37,7 @@
)
from lp.services.webapp.publisher import (
canonical_url,
+ DataDownloadView,
LaunchpadView,
)
@@ -271,17 +272,15 @@
self.updateContextFromData(data)
-class BaseRdfView:
+class BaseRdfView(DataDownloadView):
"""A view that sets its mime-type to application/rdf+xml."""
template = None
filename = None
- def __init__(self, context, request):
- self.context = context
- self.request = request
+ content_type = 'application/rdf+xml'
- def __call__(self):
+ def getBody(self):
"""Render RDF output, and return it as a string encoded in UTF-8.
Render the page template to produce RDF output.
@@ -289,10 +288,6 @@
As a side-effect, HTTP headers are set for the mime type
and filename for download."""
- self.request.response.setHeader('Content-Type', 'application/rdf+xml')
- self.request.response.setHeader(
- 'Content-Disposition', 'attachment; filename=%s.rdf' % (
- self.filename))
unicodedata = self.template()
encodeddata = unicodedata.encode('utf-8')
return encodeddata
=== 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-26 14:51:36 +0000
@@ -45,6 +45,7 @@
Link,
)
from lp.services.webapp.interfaces import ILaunchBag
+from lp.services.webapp.publisher import DataDownloadView
class SignedCodeOfConductSetNavigation(GetitemNavigation):
@@ -132,34 +133,25 @@
return self.context.title
-class CodeOfConductDownloadView:
+class CodeOfConductDownloadView(DataDownloadView):
"""Download view class for CoC page.
- This view does not use a template, but uses a __call__ method
- that returns a file to the browser.
+ This view provides a text file with "Content-disposition: attachment",
+ causing browsers to download rather than display it.
"""
- def __init__(self, context, request):
- self.context = context
- self.request = request
+ content_type = 'text/plain'
- def __call__(self):
- """Set response headers to download an attachment, and return
- CoC file data.
- """
+ def getBody(self):
# Use the context attribute 'content' as data to return.
# Avoid open the CoC file again.
- content = self.context.content
+ return self.context.content
+ @property
+ def filename(self):
# Build a fancy filename:
# - Use title with no spaces and append '.txt'
- filename = self.context.title.replace(' ', '') + '.txt'
-
- self.request.response.setHeader('Content-Type', 'application/text')
- self.request.response.setHeader('Content-Length', len(content))
- self.request.response.setHeader(
- 'Content-Disposition', 'attachment; filename="%s"' % filename)
- return content
+ return self.context.title.replace(' ', '') + '.txt'
class CodeOfConductSetView(LaunchpadView):
=== 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-26 14:51:36 +0000
@@ -1195,7 +1195,7 @@
@property
def filename(self):
- return self.context.name
+ return '%s.rdf' % self.context.name
class PersonRdfContentsView:
=== 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-26 14:51:36 +0000
@@ -1814,7 +1814,7 @@
@property
def filename(self):
- return self.context.name
+ return '%s.rdf' % self.context.name
class Icon:
=== 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-26 14:51:36 +0000
@@ -263,7 +263,7 @@
@property
def filename(self):
- return '%s-%s-%s' % (
+ return '%s-%s-%s.rdf' % (
self.context.product.name,
self.context.productseries.name,
self.context.version)
=== 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-26 14:51:36 +0000
@@ -1240,7 +1240,7 @@
@property
def filename(self):
- return '%s-%s' % (self.context.product.name, self.context.name)
+ return '%s-%s.rdf' % (self.context.product.name, self.context.name)
class ProductSeriesFileBugRedirect(LaunchpadView):
=== 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-26 14:51:36 +0000
@@ -7,9 +7,13 @@
from zope.component import getUtility
-from lp.registry.interfaces.codeofconduct import ISignedCodeOfConductSet
+from lp.registry.interfaces.codeofconduct import (
+ ICodeOfConductSet,
+ ISignedCodeOfConductSet,
+ )
from lp.registry.model.codeofconduct import SignedCodeOfConduct
from lp.testing import (
+ BrowserTestCase,
login_celebrity,
TestCaseWithFactory,
)
@@ -147,3 +151,20 @@
def test_admincomment_required(self):
self.verify_admincomment_required('Deactivate', '+deactivate')
+
+
+class TestCodeOfConductBrowser(BrowserTestCase):
+ """Test the download view for the CoC."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_response(self):
+ """Ensure the headers and body are as expected."""
+ coc = getUtility(ICodeOfConductSet)['1.1']
+ content = coc.content
+ browser = self.getViewBrowser(coc, '+download')
+ self.assertEqual(content, browser.contents)
+ self.assertEqual(str(len(content)), browser.headers['Content-length'])
+ disposition = 'attachment; filename="UbuntuCodeofConduct-1.1.txt"'
+ self.assertEqual(disposition, browser.headers['Content-disposition'])
+ self.assertEqual('text/plain', browser.headers['Content-type'])
=== 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-26 14:51:36 +0000
@@ -11,6 +11,7 @@
from lp.services.config import config
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
+ BrowserTestCase,
login_person,
TestCaseWithFactory,
)
@@ -50,3 +51,19 @@
'''))
self.assertEquals(
'http://prod.launchpad.dev/~eris', self.view.openid_identity_url)
+
+
+class TestPersonRdfView(BrowserTestCase):
+ """Test the RDF view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_headers(self):
+ """The headers for the RDF view of a person should be as expected."""
+ person = self.factory.makePerson()
+ content_disposition = 'attachment; filename="%s.rdf"' % person.name
+ browser = self.getViewBrowser(person, view_name='+rdf')
+ self.assertEqual(
+ content_disposition, browser.headers['Content-disposition'])
+ self.assertEqual(
+ 'application/rdf+xml', browser.headers['Content-type'])
=== 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-26 14:51:36 +0000
@@ -20,6 +20,7 @@
from lp.services.config import config
from lp.services.webapp.publisher import canonical_url
from lp.testing import (
+ BrowserTestCase,
login_celebrity,
login_person,
person_logged_in,
@@ -386,3 +387,19 @@
self.assertTrue(
'Y.lp.app.choice.addBinaryChoice' in str(
content.find(id='fnord-edit-license-approved').parent))
+
+
+class TestProductRdfView(BrowserTestCase):
+ """Test the Product RDF view."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_headers(self):
+ """The headers for the RDF view of a product should be as expected."""
+ product = self.factory.makeProduct()
+ browser = self.getViewBrowser(product, view_name='+rdf')
+ content_disposition = 'attachment; filename="%s.rdf"' % product.name
+ self.assertEqual(
+ content_disposition, browser.headers['Content-disposition'])
+ self.assertEqual(
+ 'application/rdf+xml', browser.headers['Content-type'])
=== 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-26 14:51:36 +0000
@@ -4,7 +4,7 @@
... GET /firefox/trunk/0.9/+rdf HTTP/1.1
... """)
HTTP/1.1 200 Ok
- Content-Disposition: attachment; filename=firefox-trunk-0.9.rdf
+ Content-Disposition: attachment; filename="firefox-trunk-0.9.rdf"
Content-Length: ...
Content-Type: application/rdf+xml
Vary: ...
=== 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-26 14:51:36 +0000
@@ -4,7 +4,7 @@
... GET /firefox/trunk/+rdf HTTP/1.1
... """)
HTTP/1.1 200 Ok
- Content-Disposition: attachment; filename=firefox-trunk.rdf
+ Content-Disposition: attachment; filename="firefox-trunk.rdf"
Content-Length: ...
Content-Type: application/rdf+xml
Vary: ...
=== 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-26 14:51:36 +0000
@@ -7,6 +7,7 @@
__metaclass__ = type
__all__ = [
+ 'DataDownloadView',
'LaunchpadContainer',
'LaunchpadView',
'LaunchpadXMLRPCView',
@@ -220,6 +221,29 @@
return cls
+class DataDownloadView:
+ """Download data without templating.
+
+ Subclasses must provide getBody, content_type and filename.
+ """
+
+ def __init__(self, context, request):
+ self.context = context
+ self.request = request
+
+ def __call__(self):
+ """Set the headers and return the body.
+
+ It is not necessary to supply Content-length, because this is added by
+ the caller.
+ """
+ self.request.response.setHeader('Content-Type', self.content_type)
+ self.request.response.setHeader(
+ 'Content-Disposition', 'attachment; filename="%s"' % (
+ self.filename))
+ return self.getBody()
+
+
class UserAttributeCache:
"""Mix in to provide self.user, cached."""
