Merge lp:~rockstar/launchpad/upgrade-branch-rc into lp:launchpad/db-devel

[Paul Hummer]

[Paul Hummer (rockstar)]

Hi-

  This branch fixes two release-critical issues with the branch upgrade job
stuff. The first issue is that IBranch.upgrade_pending didn't check to see if
the upgrade had failed or was finished. In those cases, it should return
false. The other issue is that the BranchUpgradeJob wasn't triggering a mirror
after it got upgraded, so the branch still thought it needed upgrading.

  The patch is the tests for those fixes and then the fixes.

Cheers,
Paul

[Tim Penhey (thumper)]

Looks OK to me.

review: Approve

[Gary Poster (gary)]

review: Approve (rc)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-01-22 22:25:30 +0000
+++ database/schema/security.cfg	2010-01-26 02:27:13 +0000
@@ -1716,7 +1716,7 @@
 [upgrade-branches]
 type=user
 groups=script
-public.branch                           = SELECT
+public.branch                           = SELECT, UPDATE
 public.branchjob                        = SELECT
 public.job                              = SELECT, UPDATE
 

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2010-01-22 06:03:19 +0000
+++ lib/lp/code/model/branch.py	2010-01-26 02:27:13 +0000
@@ -74,6 +74,8 @@
     IFindOfficialBranchLinks)
 from lp.registry.interfaces.person import (
     validate_person_not_private_membership, validate_public_person)
+from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.model.job import Job
 from lp.services.mail.notificationrecipientset import (
     NotificationRecipientSet)
 
@@ -1012,6 +1014,9 @@
         jobs = store.find(
             BranchJob,
             BranchJob.branch == self,
+            Job.id == BranchJob.jobID,
+            Job._status != JobStatus.COMPLETED,
+            Job._status != JobStatus.FAILED,
             BranchJob.job_type == BranchJobType.UPGRADE_BRANCH)
         return jobs.count() > 0
 

=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py	2010-01-26 01:20:54 +0000
+++ lib/lp/code/model/branchjob.py	2010-01-26 02:27:13 +0000
@@ -343,6 +343,8 @@
             upgrade_transport.delete_tree('backup.bzr')
             source_branch_transport.rename('.bzr', 'backup.bzr')
             upgrade_transport.copy_tree_to_transport(source_branch_transport)
+
+            self.branch.requestMirror()
         finally:
             shutil.rmtree(upgrade_branch_path)
 

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2010-01-22 06:03:19 +0000
+++ lib/lp/code/model/tests/test_branch.py	2010-01-26 02:27:13 +0000
@@ -65,6 +65,7 @@
 from lp.registry.model.product import ProductSet
 from lp.registry.model.sourcepackage import SourcePackage
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.job.interfaces.job import JobStatus
 from lp.testing import (
     run_with_login, TestCase, TestCaseWithFactory, time_counter)
 from lp.testing.factory import LaunchpadObjectFactory
@@ -443,6 +444,20 @@
 
         self.assertFalse(branch.upgrade_pending)
 
+    def test_upgradePending_old_job_exists(self):
+        # If the branch had an upgrade pending, but then the job was completed,
+        # then upgrade_pending should return False.
+        branch = self.factory.makeAnyBranch(
+            branch_format=BranchFormat.BZR_BRANCH_6)
+        owner = removeSecurityProxy(branch).owner
+        login_person(owner)
+        self.addCleanup(logout)
+        branch_job = removeSecurityProxy(branch.requestUpgrade())
+        branch_job.job.start()
+        branch_job.job.complete()
+
+        self.assertFalse(branch.upgrade_pending)
+
 
 class TestBzrIdentity(TestCaseWithFactory):
     """Test IBranch.bzr_identity."""

=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py	2010-01-22 06:03:19 +0000
+++ lib/lp/code/model/tests/test_branchjob.py	2010-01-26 02:27:13 +0000
@@ -8,6 +8,7 @@
 import datetime
 import os
 import shutil
+from storm.locals import Store
 import tempfile
 from unittest import TestLoader
 
@@ -263,6 +264,8 @@
             new_branch.repository._format.get_format_string(),
             'Bazaar repository format 2a (needs bzr 1.16 or later)\n')
 
+        self.assertTrue(db_branch.pending_writes)
+
     def test_needs_no_upgrading(self):
         # Branch upgrade job creation should raise an AssertionError if the
         # branch does not need to be upgraded.