Merge lp:~cjwatson/launchpad/snap-upload-check-filename into lp:launchpad

[Colin Watson]

If other files (e.g. manifests) are uploaded alongside snaps, make sure that there is still at least one snap.

If other files (e.g. manifests) are uploaded alongside snaps, make sure that there is still at least one snap.

[Maximiliano Bertacchini (maxiberta)]

Looks good. One question inline.

[William Grant (wgrant)]

review: Approve (code)

=== 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-09-16 11:59:12 +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-09-16 11:59:12 +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,22 @@
         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.assertFalse(self.build.verifySuccessfulUpload())
+
     def test_triggers_store_uploads(self):
         # The upload processor triggers store uploads if appropriate.
         self.pushConfig(