launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-api-accept-reject into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1006173 in Launchpad itself: "Replace queue tool with API"
  https://bugs.launchpad.net/launchpad/+bug/1006173

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-api-accept-reject/+merge/107894

== Summary ==

This is a small first step towards fixing bug 1006173, to provide an API sufficient to replace scripts/ftpmaster-tools/queue.

Most of the rest of the work gets fairly big and complicated, so I split off this initial branch since it has value in itself and I hope it's simple enough to be relatively uncontentious.

== Proposed fix ==

Export PU.acceptFromQueue and PU.rejectFromQueue.

I also exported PU.id while I was there, since that makes it a bit easier to emulate the "queue accept ID" form.

== LOC Rationale ==

+91.  I think this is valid because this is part of an arc of work (resourced by Ubuntu Engineering) that will culminate in removing lib/lp/soyuz/scripts/queue.py and scripts/ftpmaster-tools/queue for -862; I expect the whole thing to come in comfortably inside that.

== Tests ==

bin/test -vvct test_packageupload

== Demo and Q/A ==

I have a local implementation of a client-side queue tool that could be used to demo this on (I guess, since I need to run the uploader) dogfood, by accepting and rejecting a couple of uploads.  It's fairly easy to do this by hand in lp-shell, though: use DistroSeries.getPackageUploads() to get hold of some PackageUpload objects, and then call .acceptFromQueue() or .rejectFromQueue() on those.
-- 
https://code.launchpad.net/~cjwatson/launchpad/queue-api-accept-reject/+merge/107894
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-api-accept-reject into lp:launchpad.

=== modified file 'lib/lp/soyuz/interfaces/queue.py'
--- lib/lp/soyuz/interfaces/queue.py	2011-12-24 16:54:44 +0000
+++ lib/lp/soyuz/interfaces/queue.py	2012-05-30 00:34:28 +0000
@@ -25,7 +25,9 @@
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
     export_as_webservice_entry,
+    export_write_operation,
     exported,
+    operation_for_version,
     )
 from lazr.restful.fields import Reference
 from zope.interface import (
@@ -98,9 +100,10 @@
 
     export_as_webservice_entry(publish_web_link=False)
 
-    id = Int(
+    id = exported(
+        Int(
             title=_("ID"), required=True, readonly=True,
-            )
+            ))
 
     status = exported(
         Choice(
@@ -258,6 +261,8 @@
             has no sources associated to it.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
     def acceptFromQueue(logger=None, dry_run=False):
         """Call setAccepted, do a syncUpdate, and send notification email.
 
@@ -266,6 +271,8 @@
         :raises: AssertionError if the context is a delayed-copy.
         """
 
+    @export_write_operation()
+    @operation_for_version("devel")
     def rejectFromQueue(logger=None, dry_run=False):
         """Call setRejected, do a syncUpdate, and send notification email."""
 

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2012-05-21 07:34:15 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2012-05-30 00:34:28 +0000
@@ -8,6 +8,8 @@
 import os
 import shutil
 
+from lazr.restfulclient.errors import Unauthorized
+import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -29,6 +31,7 @@
     PackageUploadCustomFormat,
     PackageUploadStatus,
     )
+from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.interfaces.queue import (
     IPackageUploadSet,
@@ -37,9 +40,18 @@
 from lp.soyuz.interfaces.section import ISectionSet
 from lp.soyuz.model.packagecopyjob import IPackageCopyJobSource
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    admin_logged_in,
+    api_url,
+    launchpadlib_for,
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing.dbuser import switch_dbuser
-from lp.testing.layers import LaunchpadZopelessLayer
+from lp.testing.layers import (
+    LaunchpadFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
 from lp.testing.matchers import Provides
 
 
@@ -855,3 +867,75 @@
         pu.changesfile = None
         pu.rejectFromQueue()
         self.assertEqual(PackageUploadStatus.REJECTED, pu.status)
+
+
+class TestPackageUploadWebservice(TestCaseWithFactory):
+    """Test the exposure of queue methods to the web service."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestPackageUploadWebservice, self).setUp()
+        self.webservice = None
+
+    def makeDistroSeries(self):
+        self.distroseries = self.factory.makeDistroSeries()
+        self.main = self.factory.makeComponent("main")
+        self.factory.makeComponentSelection(
+            distroseries=self.distroseries, component=self.main)
+
+    def makeQueueAdmin(self, components):
+        person = self.factory.makePerson()
+        for component in components:
+            getUtility(IArchivePermissionSet).newQueueAdmin(
+                self.distroseries.main_archive, person, component)
+        return person
+
+    def load(self, obj, person=None):
+        if person is None:
+            with admin_logged_in():
+                person = self.factory.makePerson()
+        if self.webservice is None:
+            self.webservice = launchpadlib_for("testing", person)
+        return self.webservice.load(api_url(obj))
+
+    def assertRequiresEdit(self, method_name, **kwargs):
+        """Test that a web service queue method requires launchpad.Edit."""
+        with admin_logged_in():
+            upload = self.factory.makeSourcePackageUpload()
+        transaction.commit()
+        ws_upload = self.load(upload)
+        self.assertRaises(Unauthorized, getattr(ws_upload, method_name),
+                          **kwargs)
+
+    def test_edit_permissions(self):
+        self.assertRequiresEdit("acceptFromQueue")
+        self.assertRequiresEdit("rejectFromQueue")
+
+    def test_acceptFromQueue_archive_admin(self):
+        # acceptFromQueue as an archive admin accepts the upload.
+        self.makeDistroSeries()
+        person = self.makeQueueAdmin([self.main])
+        with person_logged_in(person):
+            upload = self.factory.makeSourcePackageUpload(
+                distroseries=self.distroseries, component=self.main)
+        transaction.commit()
+
+        ws_upload = self.load(upload, person)
+        self.assertEqual("New", ws_upload.status)
+        ws_upload.acceptFromQueue()
+        self.assertEqual("Done", ws_upload.status)
+
+    def test_rejectFromQueue_archive_admin(self):
+        # rejectFromQueue as an archive admin rejects the upload.
+        self.makeDistroSeries()
+        person = self.makeQueueAdmin([self.main])
+        with person_logged_in(person):
+            upload = self.factory.makeSourcePackageUpload(
+                distroseries=self.distroseries, component=self.main)
+        transaction.commit()
+
+        ws_upload = self.load(upload, person)
+        self.assertEqual("New", ws_upload.status)
+        ws_upload.rejectFromQueue()
+        self.assertEqual("Rejected", ws_upload.status)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-05-29 22:37:38 +0000
+++ lib/lp/testing/factory.py	2012-05-30 00:34:28 +0000
@@ -3509,14 +3509,14 @@
         return package_upload
 
     def makeSourcePackageUpload(self, distroseries=None,
-                                sourcepackagename=None):
+                                sourcepackagename=None, component=None):
         """Make a `PackageUpload` with a `PackageUploadSource` attached."""
         if distroseries is None:
             distroseries = self.makeDistroSeries()
         upload = self.makePackageUpload(
             distroseries=distroseries, archive=distroseries.main_archive)
         upload.addSource(self.makeSourcePackageRelease(
-            sourcepackagename=sourcepackagename))
+            sourcepackagename=sourcepackagename, component=component))
         return upload
 
     def makeBuildPackageUpload(self, distroseries=None,