Merge lp:~stevenk/launchpad/destroy-bfj-on-branch-delete into lp:launchpad

[Steve Kowalik]

Also delete BuildFarmJobs in Branch._deleteJobs() to avoid InconsistentBuildFarmJobError on Builder:+history.

Branch._deleteJobs() made the assumption that ON DELETE CASCADE would remove all references to the TTB that was being deleted. Which will remove everything but the BuildFarmJob. So delete that, and add a test that makes sure Builder:+history deals with it.

[Ian Booth (wallyworld)]

Looks ok, but bear in mind I am not an expert in this area.

review: Approve

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2012-09-14 01:21:13 +0000
+++ lib/lp/code/model/branch.py	2012-09-18 04:16:18 +0000
@@ -65,6 +65,7 @@
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
 from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.bzr import (
     BranchFormat,
@@ -1252,12 +1253,20 @@
         # the affected Jobs in the database otherwise.
         store.find(BuildQueue, BuildQueue.jobID.is_in(affected_jobs)).remove()
 
+        # Find BuildFarmJobs to delete.
+        bfjs = store.find(
+            (BuildFarmJob.id,),
+            TranslationTemplatesBuild.build_farm_job_id == BuildFarmJob.id,
+            TranslationTemplatesBuild.branch == self)
+        bfj_ids = [bfj[0] for bfj in bfjs]
+
         # Delete Jobs.  Their BranchJobs cascade along in the database.
         store.find(Job, Job.id.is_in(affected_jobs)).remove()
 
         store.find(
             TranslationTemplatesBuild,
             TranslationTemplatesBuild.branch == self).remove()
+        store.find(BuildFarmJob, BuildFarmJob.id.is_in(bfj_ids)).remove()
 
     def destroySelf(self, break_references=False):
         """See `IBranch`."""

=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py	2012-08-30 21:59:39 +0000
+++ lib/lp/code/model/tests/test_branch.py	2012-09-18 04:16:18 +0000
@@ -39,6 +39,7 @@
     IBugSet,
     )
 from lp.bugs.model.bugbranch import BugBranch
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.bzr import (
     BranchFormat,
@@ -166,6 +167,9 @@
     LaunchpadZopelessLayer,
     ZopelessAppServerLayer,
     )
+from lp.translations.model.translationtemplatesbuild import (
+    TranslationTemplatesBuild,
+    )
 from lp.translations.model.translationtemplatesbuildjob import (
     ITranslationTemplatesBuildJobSource,
     )
@@ -1374,17 +1378,25 @@
         job = source.create(branch)
         other_job = source.create(other_branch)
         store = Store.of(branch)
+        bfj = store.find(
+            BuildFarmJob,
+            BuildFarmJob.id == TranslationTemplatesBuild.build_farm_job_id,
+            TranslationTemplatesBuild.branch == branch).one().id
 
         branch.destroySelf(break_references=True)
 
         # The BuildQueue for the job whose branch we deleted is gone.
         buildqueue = store.find(BuildQueue, BuildQueue.job == job.job)
-        self.assertEqual([], list(buildqueue))
+        self.assertEqual(0, buildqueue.count())
+
+        # The BuildFarmJob for the TTB is gone.
+        bfjs = store.find(BuildFarmJob, BuildFarmJob.id == bfj)
+        self.assertEqual(0, bfjs.count())
 
         # The other job's BuildQueue entry is still there.
         other_buildqueue = store.find(
             BuildQueue, BuildQueue.job == other_job.job)
-        self.assertNotEqual([], list(other_buildqueue))
+        self.assertEqual(1, other_buildqueue.count())
 
     def test_createsJobToReclaimSpace(self):
         # When a branch is deleted from the database, a job to remove the