Merge lp:~james-w/pkgme-service/spot-desktop into lp:pkgme-service

[James Westby]

Check that the extracted package has a manifest file (required for click).

Hi,

This is quite a simple fix to get an informative review message if
someone submits a non-click .deb package to the click appstore.

Thanks,

James

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

=== modified file 'src/djpkgme/tasks.py'
--- src/djpkgme/tasks.py	2014-01-29 18:34:03 +0000
+++ src/djpkgme/tasks.py	2014-03-10 15:29:03 +0000
@@ -291,10 +291,21 @@
             explanation += indent(e.output, 4)
             result['explanation'] = explanation
             dest = None
+        else:
+            if not os.path.isfile(self.manifest_path(dest)):
+                result['passed'] = False
+                result['explanation'] = (
+                    'The package has no manifest file '
+                    '(is it a desktop package?)'
+                    )
+                dest = None
         return dest, result
 
+    def manifest_path(self, package):
+        return os.path.join(package, 'DEBIAN/manifest')
+
     def get_package_manifest(self, package):
-        with open(os.path.join(package, 'DEBIAN/manifest')) as f:
+        with open(self.manifest_path(package)) as f:
             manifest_content = json.loads(f.read())
         return manifest_content
 

=== modified file 'src/djpkgme/tests/test_tasks.py'
--- src/djpkgme/tests/test_tasks.py	2014-01-29 17:59:40 +0000
+++ src/djpkgme/tests/test_tasks.py	2014-03-10 15:29:03 +0000
@@ -664,6 +664,12 @@
         tempdir_mock.__exit__ = mock.Mock(return_value=False)
         return tempdir_mock
 
+    def test_manifest_path(self):
+        basepath = 'foo'
+        job = ClickPackageReviewJob()
+        self.assertEqual(os.path.join(basepath, 'DEBIAN', 'manifest'),
+                         job.manifest_path(basepath))
+
     @patch('djpkgme.tasks.download_file')
     def test_download_package(self, mock_download_file):
         mock_download_file.return_value = 'some/path'
@@ -677,12 +683,14 @@
         mock_download_file.assert_called_with(metadata['package_url'],
             temp_dir)
 
+    @patch('djpkgme.tasks.os.path.isfile')
     @patch('djpkgme.tasks.subprocess.check_output')
-    def test_unpack_package(self, mock_call):
+    def test_unpack_package(self, mock_call, mock_isfile):
         job = ClickPackageReviewJob()
         package_file = 'some/package'
         temp_dir = 'temp/dir'
         unpack_path = os.path.join(temp_dir, 'unpacked')
+        mock_isfile.return_value = True
         returned_path, check = job.unpack_package(package_file, temp_dir)
         self.assertEqual(unpack_path, returned_path)
         self.assertEqual(
@@ -690,6 +698,9 @@
         mock_call.assert_called_with(
             ['dpkg-deb', '-R', package_file, unpack_path],
             stderr=subprocess.STDOUT)
+        mock_isfile.assert_called_with(
+            os.path.join(unpack_path, 'DEBIAN', 'manifest')
+            )
 
     @patch('djpkgme.tasks.subprocess.check_output')
     def test_unpack_package_fails(self, mock_call):
@@ -712,6 +723,24 @@
             ['dpkg-deb', '-R', package_file, unpack_path],
             stderr=subprocess.STDOUT)
 
+    @patch('djpkgme.tasks.os.path.isfile')
+    @patch('djpkgme.tasks.subprocess.check_output')
+    def test_unpack_package_no_manifest(self, mock_call, mock_isfile):
+        job = ClickPackageReviewJob()
+        package_file = 'some/package'
+        temp_dir = 'temp/dir'
+        unpack_path = os.path.join(temp_dir, 'unpacked')
+        mock_isfile.return_value = False
+        returned_path, check = job.unpack_package(package_file, temp_dir)
+        self.assertEqual(None, returned_path)
+        expected_result = dict(passed=False,
+                               description="Is a valid click package")
+        expected_result['explanation'] = (
+            "The package has no manifest file "
+            "(is it a desktop package?)"
+            )
+        self.assertEqual(check, expected_result)
+
     def test_get_manifest(self):
         job = ClickPackageJob()
         package_name = 'some.package'