Merge lp:~thomir/launchpad/devel-set-content-type into lp:launchpad

Proposed by Thomi Richards on 2015-02-01
Status: Merged
Approved by: Colin Watson on 2015-02-04
Approved revision: no longer in the source branch.
Merged at revision: 17332
Proposed branch: lp:~thomir/launchpad/devel-set-content-type
Merge into: lp:launchpad
Diff against target: 80 lines (+42/-3)
2 files modified
lib/lp/registry/browser/productrelease.py (+8/-1)
lib/lp/registry/browser/tests/test_productrelease.py (+34/-2)
To merge this branch: bzr merge lp:~thomir/launchpad/devel-set-content-type
Reviewer Review Type Date Requested Status
William Grant code 2015-02-01 Approve on 2015-02-04
Review via email: mp+248207@code.launchpad.net

Commit Message

Code tarballs and installers are now served with the application/octet-stream mimetype when the correct mimetype cannot be guessed.

Description of the Change

Files uploaded as part of a release were previously stored as text/plain when their mimetype could not be guessed from the filename alone.

This branch changes the default, so that 'code tarballs' and 'installer' release types are stored as 'application/octet-stream' when their mimetype cannot be guessed.

To post a comment you must log in.
Kit Randel (blr) wrote :

Looks good, however is application/x-gzip a more appropriate mime type for tarballs?

William Grant (wgrant) wrote :

This is the fallback for when we can't guess the MIME type from the extension, so application/octet-stream is correct.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/productrelease.py'
2--- lib/lp/registry/browser/productrelease.py 2013-04-10 08:09:05 +0000
3+++ lib/lp/registry/browser/productrelease.py 2015-02-03 21:22:24 +0000
4@@ -48,6 +48,7 @@
5 from lp.registry.interfaces.productrelease import (
6 IProductRelease,
7 IProductReleaseFileAddForm,
8+ UpstreamFileType,
9 )
10 from lp.services.webapp import (
11 canonical_url,
12@@ -303,7 +304,13 @@
13 file_upload.filename)
14
15 if content_type is None:
16- content_type = "text/plain"
17+ if filetype in (
18+ UpstreamFileType.CODETARBALL,
19+ UpstreamFileType.INSTALLER
20+ ):
21+ content_type = "application/octet-stream"
22+ else:
23+ content_type = "text/plain"
24
25 # signature_upload is u'' if no file is specified in
26 # the browser.
27
28=== modified file 'lib/lp/registry/browser/tests/test_productrelease.py'
29--- lib/lp/registry/browser/tests/test_productrelease.py 2013-01-25 05:34:59 +0000
30+++ lib/lp/registry/browser/tests/test_productrelease.py 2015-02-03 21:22:24 +0000
31@@ -20,11 +20,11 @@
32
33 layer = LaunchpadFunctionalLayer
34
35- def makeForm(self, file_name):
36+ def makeForm(self, file_name, file_release_type='CODETARBALL'):
37 upload = self.factory.makeFakeFileUpload(filename=file_name)
38 form = {
39 'field.description': 'App 0.1 tarball',
40- 'field.contenttype': 'CODETARBALL',
41+ 'field.contenttype': file_release_type,
42 'field.filecontent': upload,
43 'field.actions.add': 'Upload',
44 }
45@@ -68,3 +68,35 @@
46 release, '+adddownloadfile', form=form)
47 self.assertEqual(
48 ['Only public projects can have download files.'], view.errors)
49+
50+ def assertFileHasMimeType(self, file_release_type, expected_mimetype):
51+ """Assert that a release file is stored with a specific mimetype.
52+
53+ :param file_release_type: A string specifying the type of the release
54+ file. Must be one of: CODETARBALL, README, RELEASENOTES,
55+ CHANGELOG, INSTALLER.
56+ :param expected_mimetype: The mimetype string you expect the file to
57+ have.
58+
59+ """
60+ release = self.factory.makeProductRelease()
61+ maintainer = release.milestone.product.owner
62+ form = self.makeForm('foo', file_release_type)
63+ with person_logged_in(maintainer):
64+ view = create_initialized_view(
65+ release, '+adddownloadfile', form=form)
66+ self.assertEqual(1, release.files.count())
67+ self.assertEqual(
68+ expected_mimetype,
69+ release.files[0].libraryfile.mimetype
70+ )
71+
72+ def test_tarballs_and_installers_are_octet_stream(self):
73+ self.assertFileHasMimeType('CODETARBALL', "application/octet-stream")
74+ self.assertFileHasMimeType('INSTALLER', "application/octet-stream")
75+
76+ def test_readme_files_are_text_plain(self):
77+ self.assertFileHasMimeType('README', "text/plain")
78+ self.assertFileHasMimeType('CHANGELOG', "text/plain")
79+ self.assertFileHasMimeType('RELEASENOTES', "text/plain")
80+