launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-upload-check-filename into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1608432 in Launchpad itself: "snaps with type: os should allow publishing of .manifest files"
  https://bugs.launchpad.net/launchpad/+bug/1608432

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-upload-check-filename/+merge/304369

If other files (e.g. manifests) are uploaded alongside snaps, make sure that there is still at least one snap.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-upload-check-filename into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/snapupload.py'
--- lib/lp/archiveuploader/snapupload.py	2015-08-03 15:07:29 +0000
+++ lib/lp/archiveuploader/snapupload.py	2016-08-30 12:36:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Process a snap package upload."""
@@ -38,23 +38,27 @@
         self.logger.debug("Beginning processing.")
 
         found_snap = False
+        snap_paths = []
         for dirpath, _, filenames in os.walk(self.upload_path):
             if dirpath == self.upload_path:
                 # All relevant files will be in a subdirectory.
                 continue
             for snap_file in sorted(filenames):
-                snap_path = os.path.join(dirpath, snap_file)
-                libraryfile = self.librarian.create(
-                    snap_file, os.stat(snap_path).st_size,
-                    open(snap_path, "rb"),
-                    filenameToContentType(snap_path),
-                    restricted=build.is_private)
-                found_snap = True
-                build.addFile(libraryfile)
+                if snap_file.endswith(".snap"):
+                    found_snap = True
+                snap_paths.append(os.path.join(dirpath, snap_file))
 
         if not found_snap:
             raise UploadError("Build did not produce any snap packages.")
 
+        for snap_path in snap_paths:
+            libraryfile = self.librarian.create(
+                os.path.basename(snap_path), os.stat(snap_path).st_size,
+                open(snap_path, "rb"),
+                filenameToContentType(snap_path),
+                restricted=build.is_private)
+            build.addFile(libraryfile)
+
         # The master verifies the status to confirm successful upload.
         self.logger.debug("Updating %s" % build.title)
         build.updateStatus(BuildStatus.FULLYBUILT)

=== modified file 'lib/lp/archiveuploader/tests/test_snapupload.py'
--- lib/lp/archiveuploader/tests/test_snapupload.py	2016-06-28 21:10:18 +0000
+++ lib/lp/archiveuploader/tests/test_snapupload.py	2016-08-30 12:36:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test uploads of SnapBuilds."""
@@ -53,6 +53,7 @@
         upload_dir = os.path.join(
             self.incoming_folder, "test", str(self.build.id), "ubuntu")
         write_file(os.path.join(upload_dir, "wget_0_all.snap"), "snap")
+        write_file(os.path.join(upload_dir, "wget_0_all.manifest"), "manifest")
         handler = UploadHandler.forProcessor(
             self.uploadprocessor, self.incoming_folder, "test", self.build)
         result = handler.processSnap(self.log)
@@ -62,6 +63,23 @@
         self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
         self.assertTrue(self.build.verifySuccessfulUpload())
 
+    def test_requires_snap(self):
+        # The upload processor fails if the upload does not contain any
+        # .snap files.
+        self.assertFalse(self.build.verifySuccessfulUpload())
+        upload_dir = os.path.join(
+            self.incoming_folder, "test", str(self.build.id), "ubuntu")
+        write_file(os.path.join(upload_dir, "wget_0_all.manifest"), "manifest")
+        handler = UploadHandler.forProcessor(
+            self.uploadprocessor, self.incoming_folder, "test", self.build)
+        result = handler.processSnap(self.log)
+        self.assertEqual(UploadStatusEnum.REJECTED, result)
+        self.assertIn(
+            "ERROR Build did not produce any snap packages.",
+            self.log.getLogBuffer())
+        self.assertEqual(BuildStatus.UPLOADING, self.build.status)
+        self.assertFalse(self.build.verifySuccessfulUpload())
+
     def test_triggers_store_uploads(self):
         # The upload processor triggers store uploads if appropriate.
         self.pushConfig(