launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/fix-already-computed-pckdiff-request into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #765709 in Launchpad itself: "The exception called when a package diff is requested twice should be a BadRequest when called through the api."
  https://bugs.launchpad.net/launchpad/+bug/765709

For more details, see:
https://code.launchpad.net/~rvb/launchpad/fix-already-computed-pckdiff-request/+merge/58257

The error triggered when a package diff has already been requested (PackageDiffAlreadyRequested) must be marked as a BadRequest exception to be able to be properly handled by the calling js when the request comes from the api.

= Test =
./bin/test -cvv test_distroseriesdifference_webservice test_requestPackageDiffs_exception_already_requested

= QA =
On https://dogfood.launchpad.net/ubuntu/natty/+localpackagediffs
Try requesting the package diffs for 'adun.app', the error message should say "diff from **** to *** was already requested by ***" instead of the standard error message.
-- 
https://code.launchpad.net/~rvb/launchpad/fix-already-computed-pckdiff-request/+merge/58257
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/fix-already-computed-pckdiff-request into lp:launchpad.

=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py	2011-04-14 15:26:55 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py	2011-04-19 09:54:33 +0000
@@ -15,6 +15,7 @@
     IDistroSeriesDifferenceSource,
     )
 from lp.soyuz.enums import PackageDiffStatus
+from lp.soyuz.interfaces.packagediff import PackageDiffAlreadyRequested
 from lp.testing import (
     TestCaseWithFactory,
     ws_object,
@@ -124,6 +125,26 @@
         self.assertRaises(
             BadRequest, ws_diff.requestPackageDiffs)
 
+    def test_requestPackageDiffs_exception_already_requested(self):
+        # when the package diffs have already been requested, a call to
+        # request them again triggers a PackageDiffAlreadyRequested
+        # exception is raised.  If called through the api, a BadRequest
+        # exception is raised.
+        ds_diff = self.factory.makeDistroSeriesDifference(versions={
+            'derived': '1.2',
+            'parent': '1.3',
+            'base': '1.2'},
+            set_base_version=True)
+        ws_diff = ws_object(self.factory.makeLaunchpadService(
+            self.factory.makePerson()), ds_diff)
+        ws_diff.requestPackageDiffs()
+        transaction.commit()
+
+        self.assertRaises(
+            PackageDiffAlreadyRequested,
+            ds_diff.requestPackageDiffs, self.factory.makePerson())
+        self.assertRaises(BadRequest, ws_diff.requestPackageDiffs)
+
     def test_package_diffs(self):
         # The package diff urls exposed.
         ds_diff = self.factory.makeDistroSeriesDifference()

=== modified file 'lib/lp/soyuz/interfaces/packagediff.py'
--- lib/lp/soyuz/interfaces/packagediff.py	2011-04-19 02:36:03 +0000
+++ lib/lp/soyuz/interfaces/packagediff.py	2011-04-19 09:54:33 +0000
@@ -13,6 +13,9 @@
     'PackageDiffAlreadyRequested',
     ]
 
+import httplib
+
+from lazr.restful.declarations import webservice_error
 from zope.interface import (
     Attribute,
     Interface,
@@ -25,11 +28,15 @@
 
 from canonical.launchpad import _
 from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
-
 from lp.soyuz.enums import PackageDiffStatus
 
 
-class PackageDiffAlreadyRequested(Exception):
+class PackageDiffRequestException(Exception):
+    """Base class for package diff request errors."""
+    webservice_error(httplib.BAD_REQUEST)
+
+
+class PackageDiffAlreadyRequested(PackageDiffRequestException):
     """Raised on attempts to request an already recorded diff request. """