launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/ppa-api-errors into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #776449 in Launchpad itself: "Set Ubuntu dependencies for PPA via API"
  https://bugs.launchpad.net/launchpad/+bug/776449

For more details, see:
https://code.launchpad.net/~abentley/launchpad/ppa-api-errors/+merge/64425

= Summary =
Fix error handling for newly-exported addArchiveDependency

== Proposed fix ==
Decorate ArchiveDependencyError to use status 400.

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
bin/test-vt test_addArchiveDependency_invalid

== Demo and Q/A ==
Using the web service, add an archive as a dependency twice.  You should get a status 400, not 500.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/interfaces/archive.py
  lib/lp/soyuz/browser/tests/test_archive_webservice.py
-- 
https://code.launchpad.net/~abentley/launchpad/ppa-api-errors/+merge/64425
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/ppa-api-errors into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/tests/test_archive_webservice.py'
--- lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-06-06 14:27:47 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_webservice.py	2011-06-13 17:45:55 +0000
@@ -156,6 +156,21 @@
             dependency=ws_dependency, pocket='Release', component='main')
         self.assertContentEqual([dependency], ws_archive.dependencies)
 
+    def test_addArchiveDependency_invalid(self):
+        """Invalid requests generate a 400 status error."""
+        archive = self.factory.makeArchive()
+        dependency = self.factory.makeArchive()
+        with person_logged_in(archive.owner):
+            archive.addArchiveDependency(
+                dependency, PackagePublishingPocket.RELEASE)
+        transaction.commit()
+        ws_archive = self.wsObject(archive, archive.owner)
+        ws_dependency = self.wsObject(dependency)
+        expected_re = '(.|\n)*This dependency is already registered(.|\n)*'
+        with ExpectedException(BadRequest, expected_re):
+            ws_archive.addArchiveDependency(
+                dependency=ws_dependency, pocket='Release')
+
     def test_removeArchiveDependency_random_user(self):
         """Normal users can remove archive dependencies."""
         archive = self.factory.makeArchive()

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2011-06-11 00:49:33 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2011-06-13 17:45:55 +0000
@@ -113,6 +113,7 @@
      * It is not a PPA,
      * It is already recorded.
     """
+    webservice_error(400)  # Bad request.
 
 
 # Exceptions used in the webservice that need to be in this file to get