launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/queue-api-fix-non-component-overrides into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1006173 in Launchpad itself: "Queue tool requires direct DB access"
  https://bugs.launchpad.net/launchpad/+bug/1006173

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/queue-api-fix-non-component-overrides/+merge/114236

== Summary ==

PackageUpload.overrideSource OOPSes when passed component=None.

== Proposed fix ==

This is a simple glitch in the handling of changing the archive when the source component changes, and just needs an if guard.  There was already a test for this in the PCJ case, but not in the case of an ordinary upload; I cloned-and-hacked the PCJ test.

== LOC Rationale ==

+13.   This is part of removing the queue tool, which will get me ~1000 lines of net credit.

== Tests ==

bin/test -vvct PackageUploadTestCase.test_overrideSource_ignores_None_component_change

== Demo and Q/A ==

Using lp:ubuntu-archive-tools, run something like 'queue -l dogfood -x some-section override some-source-package'.
-- 
https://code.launchpad.net/~cjwatson/launchpad/queue-api-fix-non-component-overrides/+merge/114236
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/queue-api-fix-non-component-overrides into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py	2012-07-06 16:17:57 +0000
+++ lib/lp/soyuz/model/queue.py	2012-07-10 17:42:25 +0000
@@ -1065,8 +1065,10 @@
 
         # We override our own archive too, as it is used to create
         # the SPPH during publish().
-        self.archive = self.distroseries.distribution.getArchiveByComponent(
-            new_component.name)
+        if new_component is not None:
+            distribution = self.distroseries.distribution
+            self.archive = distribution.getArchiveByComponent(
+                new_component.name)
 
         return made_changes
 

=== modified file 'lib/lp/soyuz/tests/test_packageupload.py'
--- lib/lp/soyuz/tests/test_packageupload.py	2012-07-09 17:15:52 +0000
+++ lib/lp/soyuz/tests/test_packageupload.py	2012-07-10 17:42:25 +0000
@@ -395,6 +395,17 @@
         [spph] = upload.realiseUpload()
         self.assertEqual(spph.packageupload, upload)
 
+    def test_overrideSource_ignores_None_component_change(self):
+        # overrideSource accepts None as a component; it will not object
+        # based on permissions for the new component.
+        upload = self.factory.makeSourcePackageUpload()
+        spr = upload.sourcepackagerelease
+        current_component = spr.component
+        new_section = self.factory.makeSection()
+        upload.overrideSource(None, new_section, [current_component])
+        self.assertEqual(current_component, spr.component)
+        self.assertEqual(new_section, spr.section)
+
 
 class TestPackageUploadPrivacy(TestCaseWithFactory):
     """Test PackageUpload security."""