launchpad-reviewers team mailing list archive

[Thomi Richards <thomi.richards@xxxxxxxxxxxxx>]

Thomi Richards has proposed merging lp:~thomir/launchpad/devel-set-content-type into lp:launchpad.

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1325642 in Launchpad itself: "Unknown ProductReleaseFile types assumed to be text/plain"
  https://bugs.launchpad.net/launchpad/+bug/1325642

For more details, see:
https://code.launchpad.net/~thomir/launchpad/devel-set-content-type/+merge/248207

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. 
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thomir/launchpad/devel-set-content-type into lp:launchpad.

=== modified file 'lib/lp/registry/browser/productrelease.py'
--- lib/lp/registry/browser/productrelease.py	2013-04-10 08:09:05 +0000
+++ lib/lp/registry/browser/productrelease.py	2015-02-03 21:22:24 +0000
@@ -48,6 +48,7 @@
 from lp.registry.interfaces.productrelease import (
     IProductRelease,
     IProductReleaseFileAddForm,
+    UpstreamFileType,
     )
 from lp.services.webapp import (
     canonical_url,
@@ -303,7 +304,13 @@
                 file_upload.filename)
 
             if content_type is None:
-                content_type = "text/plain"
+                if filetype in (
+                    UpstreamFileType.CODETARBALL,
+                    UpstreamFileType.INSTALLER
+                ):
+                    content_type = "application/octet-stream"
+                else:
+                    content_type = "text/plain"
 
             # signature_upload is u'' if no file is specified in
             # the browser.

=== modified file 'lib/lp/registry/browser/tests/test_productrelease.py'
--- lib/lp/registry/browser/tests/test_productrelease.py	2013-01-25 05:34:59 +0000
+++ lib/lp/registry/browser/tests/test_productrelease.py	2015-02-03 21:22:24 +0000
@@ -20,11 +20,11 @@
 
     layer = LaunchpadFunctionalLayer
 
-    def makeForm(self, file_name):
+    def makeForm(self, file_name, file_release_type='CODETARBALL'):
         upload = self.factory.makeFakeFileUpload(filename=file_name)
         form = {
             'field.description': 'App 0.1 tarball',
-            'field.contenttype': 'CODETARBALL',
+            'field.contenttype': file_release_type,
             'field.filecontent': upload,
             'field.actions.add': 'Upload',
             }
@@ -68,3 +68,35 @@
                 release, '+adddownloadfile', form=form)
         self.assertEqual(
             ['Only public projects can have download files.'], view.errors)
+
+    def assertFileHasMimeType(self, file_release_type, expected_mimetype):
+        """Assert that a release file is stored with a specific mimetype.
+
+        :param file_release_type: A string specifying the type of the release
+            file. Must be one of: CODETARBALL, README, RELEASENOTES,
+            CHANGELOG, INSTALLER.
+        :param expected_mimetype: The mimetype string you expect the file to
+            have.
+
+        """
+        release = self.factory.makeProductRelease()
+        maintainer = release.milestone.product.owner
+        form = self.makeForm('foo', file_release_type)
+        with person_logged_in(maintainer):
+            view = create_initialized_view(
+                release, '+adddownloadfile', form=form)
+        self.assertEqual(1, release.files.count())
+        self.assertEqual(
+            expected_mimetype,
+            release.files[0].libraryfile.mimetype
+        )
+
+    def test_tarballs_and_installers_are_octet_stream(self):
+        self.assertFileHasMimeType('CODETARBALL', "application/octet-stream")
+        self.assertFileHasMimeType('INSTALLER', "application/octet-stream")
+
+    def test_readme_files_are_text_plain(self):
+        self.assertFileHasMimeType('README', "text/plain")
+        self.assertFileHasMimeType('CHANGELOG', "text/plain")
+        self.assertFileHasMimeType('RELEASENOTES', "text/plain")
+