Merge lp:~cjwatson/launchpad/queue-api-accept-reject into lp:launchpad

[Colin Watson]

Export PackageUpload.id, PackageUpload.acceptFromQueue, and PackageUpload.rejectFromQueue.

== 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 -t xx-packageupload.txt

== 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.

[Steve Kowalik (stevenk)]

This looks like excellent work and I certainly approve of your overarching plan to nuke scripts/ftpmaster-tools/queue from orbit.

review: Approve (code)

=== 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 14:16:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -22,10 +22,15 @@
     'QueueStateWriteProtectedError',
     ]
 
+import httplib
+
 from lazr.enum import DBEnumeratedType
 from lazr.restful.declarations import (
+    error_status,
     export_as_webservice_entry,
+    export_write_operation,
     exported,
+    operation_for_version,
     )
 from lazr.restful.fields import Reference
 from zope.interface import (
@@ -53,6 +58,7 @@
     """
 
 
+@error_status(httplib.BAD_REQUEST)
 class QueueInconsistentStateError(Exception):
     """Queue state machine error.
 
@@ -98,9 +104,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 +265,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 +275,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/stories/webservice/xx-packageupload.txt'
--- lib/lp/soyuz/stories/webservice/xx-packageupload.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/stories/webservice/xx-packageupload.txt	2012-05-30 14:16:22 +0000
@@ -25,6 +25,7 @@
     display_name: u'mozilla-firefox'
     display_version: u'0.9'
     distroseries_link: u'http://.../ubuntu/warty'
+    id: 11
     pocket: u'Release'
     resource_type_link: u'http://.../#package_upload'
     self_link: u'http://.../ubuntu/warty/+upload/11'

=== 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 14:16:22 +0000
@@ -8,6 +8,11 @@
 import os
 import shutil
 
+from lazr.restfulclient.errors import (
+    BadRequest,
+    Unauthorized,
+    )
+import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -29,6 +34,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 +43,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 +870,89 @@
         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_double_accept_raises_BadRequest(self):
+        # Trying to accept an upload twice returns 400 instead of OOPSing.
+        self.makeDistroSeries()
+        person = self.makeQueueAdmin([self.main])
+        with person_logged_in(person):
+            upload = self.factory.makeSourcePackageUpload(
+                distroseries=self.distroseries, component=self.main)
+            upload.setAccepted()
+        transaction.commit()
+
+        ws_upload = self.load(upload, person)
+        self.assertEqual("Accepted", ws_upload.status)
+        self.assertRaises(BadRequest, ws_upload.acceptFromQueue)
+
+    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-30 00:36:29 +0000
+++ lib/lp/testing/factory.py	2012-05-30 14:16:22 +0000
@@ -3511,14 +3511,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,