Merge lp:~cjwatson/launchpad/branch-delete-job into lp:launchpad

Proposed by Colin Watson
Status: Rejected
Rejected by: Colin Watson
Proposed branch: lp:~cjwatson/launchpad/branch-delete-job
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/code-delete-no-recipe-preload
Diff against target: 948 lines (+377/-48)
11 files modified
database/schema/security.cfg (+48/-0)
lib/lp/code/configure.zcml (+10/-1)
lib/lp/code/enums.py (+12/-1)
lib/lp/code/interfaces/branch.py (+7/-1)
lib/lp/code/interfaces/branchjob.py (+19/-1)
lib/lp/code/model/branch.py (+19/-1)
lib/lp/code/model/branchjob.py (+84/-3)
lib/lp/code/model/tests/test_branch.py (+76/-38)
lib/lp/code/model/tests/test_branchjob.py (+95/-2)
lib/lp/code/stories/webservice/xx-branch.txt (+1/-0)
lib/lp/services/config/schema-lazr.conf (+6/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/branch-delete-job
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+364907@code.launchpad.net

Commit message

Add a branch deletion job.

Description of the change

Nothing creates these jobs yet; that will come in a follow-up branch.

We'll need to get the branch-delete-job DB user created and .pgpass updated on the relevant machines before landing this.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

This seems unnecessary since adding some more indexes has made deletion fast enough, so withdrawing this until we have a clear need.

Unmerged revisions

18912. By Colin Watson

Add a branch deletion job.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2019-03-11 14:51:16 +0000
3+++ database/schema/security.cfg 2019-03-21 15:55:38 +0000
4@@ -2593,3 +2593,51 @@
5 public.teamparticipation = SELECT
6 public.webhook = SELECT
7 public.webhookjob = SELECT, INSERT
8+
9+[branch-delete-job]
10+type=user
11+groups=script
12+public.accessartifact = SELECT, DELETE
13+public.accessartifactgrant = SELECT, DELETE
14+public.accesspolicyartifact = SELECT, DELETE
15+public.archive = SELECT
16+public.branch = SELECT, UPDATE, DELETE
17+public.branchjob = SELECT, INSERT, DELETE
18+public.branchmergeproposal = SELECT, UPDATE, DELETE
19+public.branchmergeproposaljob = SELECT, DELETE
20+public.branchsubscription = SELECT, DELETE
21+public.bug = SELECT
22+public.bugbranch = SELECT, DELETE
23+public.bugtask = SELECT
24+public.buildfarmjob = SELECT, DELETE
25+public.buildqueue = SELECT, DELETE
26+public.codeimport = SELECT, DELETE
27+public.codeimportjob = SELECT, DELETE
28+public.codereviewinlinecomment = SELECT, DELETE
29+public.codereviewinlinecommentdraft = SELECT, DELETE
30+public.codereviewmessage = SELECT, DELETE
31+public.codereviewvote = SELECT, DELETE
32+public.distribution = SELECT
33+public.distributionsourcepackage = SELECT, DELETE
34+public.distroseries = SELECT
35+public.emailaddress = SELECT
36+public.job = SELECT, INSERT, UPDATE, DELETE
37+public.person = SELECT
38+public.previewdiff = SELECT, DELETE
39+public.product = SELECT
40+public.productseries = SELECT, UPDATE
41+public.seriessourcepackagebranch = SELECT, DELETE
42+public.snap = SELECT, UPDATE
43+public.sourcepackage = SELECT
44+public.sourcepackagename = SELECT
45+public.sourcepackagepublishinghistory = SELECT
46+public.sourcepackagerecipe = SELECT, DELETE
47+public.sourcepackagerecipebuild = SELECT, UPDATE
48+public.sourcepackagerecipedata = SELECT, DELETE
49+public.sourcepackagerecipedatainstruction = SELECT, DELETE
50+public.sourcepackagerecipedistroseries = SELECT, DELETE
51+public.sourcepackagerelease = SELECT
52+public.specificationbranch = SELECT, DELETE
53+public.translationtemplatesbuild = SELECT, DELETE
54+public.webhook = SELECT, DELETE
55+public.webhookjob = SELECT, DELETE
56
57=== modified file 'lib/lp/code/configure.zcml'
58--- lib/lp/code/configure.zcml 2018-10-16 15:52:00 +0000
59+++ lib/lp/code/configure.zcml 2019-03-21 15:55:38 +0000
60@@ -1,4 +1,4 @@
61-<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
62+<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
63 GNU Affero General Public License version 3 (see the file LICENSE).
64 -->
65
66@@ -602,6 +602,10 @@
67 <allow interface="lp.code.interfaces.branchjob.IBranchModifiedMailJob"/>
68 <allow interface="lp.code.interfaces.branchjob.IBranchJob"/>
69 </class>
70+ <class class="lp.code.model.branchjob.BranchDeleteJob">
71+ <allow interface="lp.code.interfaces.branchjob.IBranchDeleteJob"/>
72+ <allow interface="lp.code.interfaces.branchjob.IBranchJob"/>
73+ </class>
74 <securedutility
75 component="lp.code.model.branchjob.RevisionMailJob"
76 provides="lp.code.interfaces.branchjob.IRevisionMailJobSource">
77@@ -627,6 +631,11 @@
78 provides="lp.code.interfaces.branchjob.IBranchModifiedMailJobSource">
79 <allow interface="lp.code.interfaces.branchjob.IBranchModifiedMailJobSource"/>
80 </securedutility>
81+ <securedutility
82+ component="lp.code.model.branchjob.BranchDeleteJob"
83+ provides="lp.code.interfaces.branchjob.IBranchDeleteJobSource">
84+ <allow interface="lp.code.interfaces.branchjob.IBranchDeleteJobSource"/>
85+ </securedutility>
86
87 <!-- CodeImport -->
88
89
90=== modified file 'lib/lp/code/enums.py'
91--- lib/lp/code/enums.py 2018-10-19 23:10:41 +0000
92+++ lib/lp/code/enums.py 2019-03-21 15:55:38 +0000
93@@ -1,10 +1,11 @@
94-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
95+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
96 # GNU Affero General Public License version 3 (see the file LICENSE).
97
98 """Enumerations used in the lp/code modules."""
99
100 __metaclass__ = type
101 __all__ = [
102+ 'BranchDeletionStatus',
103 'BranchLifecycleStatus',
104 'BranchLifecycleStatusFilter',
105 'BranchListingSort',
106@@ -116,6 +117,16 @@
107 """)
108
109
110+class BranchDeletionStatus(DBEnumeratedType):
111+ """Branch Deletion Status
112+
113+ The deletion status of a branch is used to track asynchronous deletion.
114+ """
115+
116+ ACTIVE = DBItem(0, "Active")
117+ DELETING = DBItem(1, "Deleting")
118+
119+
120 class GitRepositoryType(DBEnumeratedType):
121 """Git Repository Type
122
123
124=== modified file 'lib/lp/code/interfaces/branch.py'
125--- lib/lp/code/interfaces/branch.py 2019-01-23 17:13:48 +0000
126+++ lib/lp/code/interfaces/branch.py 2019-03-21 15:55:38 +0000
127@@ -1,4 +1,4 @@
128-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
129+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
130 # GNU Affero General Public License version 3 (see the file LICENSE).
131
132 """Branch interfaces."""
133@@ -75,6 +75,7 @@
134 RepositoryFormat,
135 )
136 from lp.code.enums import (
137+ BranchDeletionStatus,
138 BranchLifecycleStatus,
139 BranchMergeProposalStatus,
140 BranchSubscriptionDiffSize,
141@@ -293,6 +294,11 @@
142 accepted).
143 """
144
145+ deletion_status = exported(Choice(
146+ title=_("Deletion status"), required=True, readonly=True,
147+ vocabulary=BranchDeletionStatus,
148+ description=_("The deletion status of this branch.")))
149+
150 # People attributes
151 registrant = exported(
152 PublicPersonChoice(
153
154=== modified file 'lib/lp/code/interfaces/branchjob.py'
155--- lib/lp/code/interfaces/branchjob.py 2015-09-01 17:10:46 +0000
156+++ lib/lp/code/interfaces/branchjob.py 2019-03-21 15:55:38 +0000
157@@ -1,4 +1,4 @@
158-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
159+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
160 # GNU Affero General Public License version 3 (see the file LICENSE).
161
162 """BranchJob interfaces."""
163@@ -8,6 +8,8 @@
164
165
166 __all__ = [
167+ 'IBranchDeleteJob',
168+ 'IBranchDeleteJobSource',
169 'IBranchJob',
170 'IBranchModifiedMailJob',
171 'IBranchModifiedMailJobSource',
172@@ -201,3 +203,19 @@
173 :param user: The `IPerson` who modified the branch.
174 :param branch_delta: An `IBranchDelta` describing the changes.
175 """
176+
177+
178+class IBranchDeleteJob(IRunnableJob):
179+ """A Job that deletes a branch from the database."""
180+
181+ branch_id = Int(title=_('The id of the branch to delete.'))
182+
183+
184+class IBranchDeleteJobSource(IJobSource):
185+
186+ def create(branch, requester):
187+ """Delete a branch from the database.
188+
189+ :param branch: The `IBranch` to delete.
190+ :param requester: The `IPerson` who requested the deletion.
191+ """
192
193=== modified file 'lib/lp/code/model/branch.py'
194--- lib/lp/code/model/branch.py 2019-03-21 15:55:38 +0000
195+++ lib/lp/code/model/branch.py 2019-03-21 15:55:38 +0000
196@@ -1,4 +1,4 @@
197-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
198+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
199 # GNU Affero General Public License version 3 (see the file LICENSE).
200
201 __metaclass__ = type
202@@ -82,6 +82,7 @@
203 RepositoryFormat,
204 )
205 from lp.code.enums import (
206+ BranchDeletionStatus,
207 BranchLifecycleStatus,
208 BranchMergeProposalStatus,
209 BranchType,
210@@ -304,6 +305,23 @@
211 # such subscriptions.
212 getUtility(IRemoveArtifactSubscriptionsJobSource).create(who, [self])
213
214+ _deletion_status = EnumCol(
215+ dbName='deletion_status', enum=BranchDeletionStatus,
216+ default=BranchDeletionStatus.ACTIVE)
217+
218+ @property
219+ def deletion_status(self):
220+ # XXX cjwatson 2019-03-19: Remove once this column has been
221+ # backfilled.
222+ if self._deletion_status is None:
223+ return BranchDeletionStatus.ACTIVE
224+ else:
225+ return self._deletion_status
226+
227+ @deletion_status.setter
228+ def deletion_status(self, value):
229+ self._deletion_status = value
230+
231 registrant = ForeignKey(
232 dbName='registrant', foreignKey='Person',
233 storm_validator=validate_public_person, notNull=True)
234
235=== modified file 'lib/lp/code/model/branchjob.py'
236--- lib/lp/code/model/branchjob.py 2018-03-30 20:42:14 +0000
237+++ lib/lp/code/model/branchjob.py 2019-03-21 15:55:38 +0000
238@@ -1,7 +1,8 @@
239-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
240+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
241 # GNU Affero General Public License version 3 (see the file LICENSE).
242
243 __all__ = [
244+ 'BranchDeleteJob',
245 'BranchJob',
246 'BranchModifiedMailJob',
247 'BranchScanJob',
248@@ -56,17 +57,22 @@
249 implementer,
250 provider,
251 )
252+from zope.security.proxy import removeSecurityProxy
253
254 from lp.code.bzr import (
255 branch_revision_history,
256 get_branch_formats,
257 )
258 from lp.code.enums import (
259+ BranchDeletionStatus,
260 BranchMergeProposalStatus,
261 BranchSubscriptionDiffSize,
262 BranchSubscriptionNotificationLevel,
263 )
264+from lp.code.errors import CannotDeleteBranch
265 from lp.code.interfaces.branchjob import (
266+ IBranchDeleteJob,
267+ IBranchDeleteJobSource,
268 IBranchJob,
269 IBranchModifiedMailJob,
270 IBranchModifiedMailJobSource,
271@@ -82,6 +88,7 @@
272 IRosettaUploadJob,
273 IRosettaUploadJobSource,
274 )
275+from lp.code.interfaces.branchlookup import IBranchLookup
276 from lp.code.mail.branch import BranchMailer
277 from lp.code.model.branch import Branch
278 from lp.code.model.branchmergeproposal import BranchMergeProposal
279@@ -121,6 +128,7 @@
280 BaseRunnableJobSource,
281 )
282 from lp.services.mail.sendmail import format_address_for_person
283+from lp.services.scripts import log
284 from lp.services.utils import text_delta
285 from lp.services.webapp import canonical_url
286 from lp.translations.interfaces.translationimportqueue import (
287@@ -195,6 +203,12 @@
288 This job runs against a branch to send emails about modifications.
289 """)
290
291+ DELETE_BRANCH = DBItem(9, """
292+ Delete branch
293+
294+ This job deletes a branch from the database.
295+ """)
296+
297
298 @implementer(IBranchJob)
299 class BranchJob(SQLBase):
300@@ -295,7 +309,9 @@
301 vars.extend([
302 ('branch_job_id', self.context.id),
303 ('branch_job_type', self.context.job_type.title)])
304- if self.context.branch is not None:
305+ if 'branch_name' in self.metadata:
306+ vars.append(('branch_name', self.metadata['branch_name']))
307+ elif self.context.branch is not None:
308 vars.append(('branch_name', self.context.branch.unique_name))
309 return vars
310
311@@ -347,7 +363,6 @@
312
313 def run(self):
314 """See `IBranchScanJob`."""
315- from lp.services.scripts import log
316 with server(get_ro_server(), no_replace=True):
317 try:
318 with try_advisory_lock(
319@@ -1056,3 +1071,69 @@
320 def run(self):
321 """See `IBranchModifiedMailJob`."""
322 self.getMailer().sendAll()
323+
324+
325+@implementer(IBranchDeleteJob)
326+@provider(IBranchDeleteJobSource)
327+class BranchDeleteJob(BranchJobDerived):
328+ """A Job that deletes a branch from the database."""
329+
330+ class_job_type = BranchJobType.DELETE_BRANCH
331+
332+ user_error_types = (CannotDeleteBranch,)
333+
334+ config = config.IBranchDeleteJobSource
335+
336+ def __repr__(self):
337+ return '<DELETE_BRANCH branch job (%(id)s) for %(branch)s>' % {
338+ 'id': self.context.id,
339+ 'branch': self.branch_name,
340+ }
341+
342+ def getOperationDescription(self):
343+ return 'deleting a branch'
344+
345+ @classmethod
346+ def create(cls, branch, requester):
347+ """See `IBranchDeleteJobSource`."""
348+ metadata = {'branch_id': branch.id, 'branch_name': branch.unique_name}
349+ # The BranchJob has a branch of None, because we don't want to
350+ # delete this job while trying to delete the branch.
351+ branch_job = BranchJob(
352+ None, cls.class_job_type, metadata, requester=requester)
353+ job = cls(branch_job)
354+ job.celeryRunOnCommit()
355+ return job
356+
357+ @property
358+ def branch_id(self):
359+ return self.metadata['branch_id']
360+
361+ @property
362+ def branch_name(self):
363+ return self.metadata['branch_name']
364+
365+ def run(self):
366+ """See `IBranchDeleteJob`."""
367+ branch = getUtility(IBranchLookup).get(self.branch_id)
368+ if branch is None:
369+ log.info(
370+ 'Skipping branch %s because it has already been deleted.' %
371+ self.branch_name)
372+ elif branch.deletion_status != BranchDeletionStatus.DELETING:
373+ log.warning(
374+ 'Skipping branch %s because its deletion status is not '
375+ 'DELETING.' % self.branch_name)
376+ else:
377+ try:
378+ branch.destroySelf(break_references=True)
379+ except CannotDeleteBranch:
380+ # Set the deletion status back to ACTIVE so that it's
381+ # possible to try again. We don't attempt to undo the
382+ # renaming at the moment. Do this in its own transaction
383+ # since the job runner will abort the transaction.
384+ transaction.abort()
385+ removeSecurityProxy(branch).deletion_status = (
386+ BranchDeletionStatus.ACTIVE)
387+ transaction.commit()
388+ raise
389
390=== modified file 'lib/lp/code/model/tests/test_branch.py'
391--- lib/lp/code/model/tests/test_branch.py 2019-01-28 18:09:21 +0000
392+++ lib/lp/code/model/tests/test_branch.py 2019-03-21 15:55:38 +0000
393@@ -1,4 +1,4 @@
394-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
395+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
396 # GNU Affero General Public License version 3 (see the file LICENSE).
397
398 """Tests for Branches."""
399@@ -1247,7 +1247,8 @@
400 "deleted.")
401 branch_id = self.branch.id
402 branch_set = getUtility(IBranchLookup)
403- self.branch.destroySelf()
404+ with dbuser(config.IBranchDeleteJobSource.dbuser):
405+ self.branch.destroySelf()
406 self.assertIsNone(
407 branch_set.get(branch_id), "The branch has not been deleted.")
408
409@@ -1280,7 +1281,8 @@
410 bug.linkBranch(self.branch, self.user)
411 self.assertEqual(self.branch.canBeDeleted(), False,
412 "A branch linked to a bug is not deletable.")
413- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
414+ with dbuser(config.IBranchDeleteJobSource.dbuser):
415+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
416
417 def test_specBranchLinkDisablesDeletion(self):
418 """A branch linked to a spec cannot be deleted."""
419@@ -1291,7 +1293,8 @@
420 spec.linkBranch(self.branch, self.user)
421 self.assertEqual(self.branch.canBeDeleted(), False,
422 "A branch linked to a spec is not deletable.")
423- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
424+ with dbuser(config.IBranchDeleteJobSource.dbuser):
425+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
426
427 def test_associatedProductSeriesBranchDisablesDeletion(self):
428 """A branch linked as a branch to a product series cannot be
429@@ -1301,14 +1304,16 @@
430 self.assertEqual(self.branch.canBeDeleted(), False,
431 "A branch that is a user branch for a product series"
432 " is not deletable.")
433- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
434+ with dbuser(config.IBranchDeleteJobSource.dbuser):
435+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
436
437 def test_productSeriesTranslationsBranchDisablesDeletion(self):
438 self.product.development_focus.translations_branch = self.branch
439 self.assertEqual(self.branch.canBeDeleted(), False,
440 "A branch that is a translations branch for a "
441 "product series is not deletable.")
442- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
443+ with dbuser(config.IBranchDeleteJobSource.dbuser):
444+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
445
446 def test_revisionsDeletable(self):
447 """A branch that has some revisions can be deleted."""
448@@ -1321,7 +1326,8 @@
449 self.assertEqual(self.branch.canBeDeleted(), True,
450 "A branch that has a revision is deletable.")
451 unique_name = self.branch.unique_name
452- self.branch.destroySelf()
453+ with dbuser(config.IBranchDeleteJobSource.dbuser):
454+ self.branch.destroySelf()
455 # Commit again to trigger the deferred indices.
456 transaction.commit()
457 branch_lookup = getUtility(IBranchLookup)
458@@ -1335,7 +1341,8 @@
459 self.branch.addLandingTarget(self.user, target_branch)
460 self.assertEqual(self.branch.canBeDeleted(), False,
461 "A branch with a landing target is not deletable.")
462- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
463+ with dbuser(config.IBranchDeleteJobSource.dbuser):
464+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
465
466 def test_landingCandidateDisablesDeletion(self):
467 """A branch with a landing candidate cannot be deleted."""
468@@ -1345,7 +1352,8 @@
469 self.assertEqual(self.branch.canBeDeleted(), False,
470 "A branch with a landing candidate is not"
471 " deletable.")
472- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
473+ with dbuser(config.IBranchDeleteJobSource.dbuser):
474+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
475
476 def test_prerequisiteBranchDisablesDeletion(self):
477 """A branch that is a prerequisite branch cannot be deleted."""
478@@ -1357,14 +1365,16 @@
479 self.assertEqual(self.branch.canBeDeleted(), False,
480 "A branch with a prerequisite target is not "
481 "deletable.")
482- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
483+ with dbuser(config.IBranchDeleteJobSource.dbuser):
484+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
485
486 def test_relatedBranchJobsDeleted(self):
487 # A branch with an associated branch job will delete those jobs.
488 branch = self.factory.makeBranch(
489 branch_format=BranchFormat.BZR_BRANCH_6)
490 removeSecurityProxy(branch).requestUpgrade(branch.owner)
491- branch.destroySelf()
492+ with dbuser(config.IBranchDeleteJobSource.dbuser):
493+ branch.destroySelf()
494 # Need to commit the transaction to fire off the constraint checks.
495 transaction.commit()
496
497@@ -1373,7 +1383,8 @@
498 # should be cleared.
499 dev_focus = self.branch.product.development_focus
500 dev_focus.translations_branch = self.branch
501- self.branch.destroySelf(break_references=True)
502+ with dbuser(config.IBranchDeleteJobSource.dbuser):
503+ self.branch.destroySelf(break_references=True)
504
505 def test_related_TranslationTemplatesBuild_cleaned_out(self):
506 # A TranslationTemplatesBuild may come with a BuildQueue entry.
507@@ -1381,7 +1392,8 @@
508 # remove the TTB.
509 build = self.factory.makeTranslationTemplatesBuild()
510 build.queueBuild()
511- build.branch.destroySelf(break_references=True)
512+ with dbuser(config.IBranchDeleteJobSource.dbuser):
513+ build.branch.destroySelf(break_references=True)
514
515 def test_unrelated_TranslationTemplatesBuild_intact(self):
516 # No innocent BuildQueue entries are harmed in deleting a
517@@ -1391,7 +1403,8 @@
518 other_build = self.factory.makeTranslationTemplatesBuild()
519 other_bq = other_build.queueBuild()
520
521- build.branch.destroySelf(break_references=True)
522+ with dbuser(config.IBranchDeleteJobSource.dbuser):
523+ build.branch.destroySelf(break_references=True)
524
525 store = Store.of(build)
526 # The BuildQueue for the job whose branch we deleted is gone.
527@@ -1406,7 +1419,8 @@
528 branch = self.factory.makeAnyBranch()
529 branch_id = branch.id
530 store = Store.of(branch)
531- branch.destroySelf()
532+ with dbuser(config.IBranchDeleteJobSource.dbuser):
533+ branch.destroySelf()
534 jobs = store.find(
535 BranchJob,
536 BranchJob.job_type == BranchJobType.RECLAIM_BRANCH_SPACE)
537@@ -1417,7 +1431,8 @@
538 def test_destroySelf_with_SourcePackageRecipe(self):
539 """If branch is a base_branch in a recipe, it is deleted."""
540 recipe = self.factory.makeSourcePackageRecipe()
541- recipe.base_branch.destroySelf(break_references=True)
542+ with dbuser(config.IBranchDeleteJobSource.dbuser):
543+ recipe.base_branch.destroySelf(break_references=True)
544
545 def test_destroySelf_with_SourcePackageRecipe_as_non_base(self):
546 """If branch is referred to by a recipe, it is deleted."""
547@@ -1425,7 +1440,8 @@
548 branch2 = self.factory.makeAnyBranch()
549 self.factory.makeSourcePackageRecipe(
550 branches=[branch1, branch2])
551- branch2.destroySelf(break_references=True)
552+ with dbuser(config.IBranchDeleteJobSource.dbuser):
553+ branch2.destroySelf(break_references=True)
554
555 def test_destroySelf_with_inline_comments_draft(self):
556 # Draft inline comments related to a deleted branch (source
557@@ -1439,7 +1455,8 @@
558 previewdiff_id=preview_diff.id,
559 person=self.user,
560 comments={'1': 'Should vanish.'})
561- self.branch.destroySelf(break_references=True)
562+ with dbuser(config.IBranchDeleteJobSource.dbuser):
563+ self.branch.destroySelf(break_references=True)
564
565 def test_destroySelf_with_inline_comments_published(self):
566 # Published inline comments related to a deleted branch (source
567@@ -1455,12 +1472,14 @@
568 previewdiff_id=preview_diff.id,
569 inline_comments={'1': 'Must disappear.'},
570 )
571- self.branch.destroySelf(break_references=True)
572+ with dbuser(config.IBranchDeleteJobSource.dbuser):
573+ self.branch.destroySelf(break_references=True)
574
575 def test_related_webhooks_deleted(self):
576 webhook = self.factory.makeWebhook(target=self.branch)
577 webhook.ping()
578- self.branch.destroySelf()
579+ with dbuser(config.IBranchDeleteJobSource.dbuser):
580+ self.branch.destroySelf()
581 transaction.commit()
582 self.assertRaises(LostObjectError, getattr, webhook, 'target')
583
584@@ -1541,7 +1560,8 @@
585 merge_proposal1, merge_proposal2 = self.makeMergeProposals()
586 merge_proposal1_id = merge_proposal1.id
587 BranchMergeProposal.get(merge_proposal1_id)
588- self.branch.destroySelf(break_references=True)
589+ with dbuser(config.IBranchDeleteJobSource.dbuser):
590+ self.branch.destroySelf(break_references=True)
591 self.assertRaises(SQLObjectNotFound,
592 BranchMergeProposal.get, merge_proposal1_id)
593
594@@ -1550,14 +1570,17 @@
595 merge_proposal1, merge_proposal2 = self.makeMergeProposals()
596 merge_proposal1_id = merge_proposal1.id
597 BranchMergeProposal.get(merge_proposal1_id)
598- merge_proposal1.target_branch.destroySelf(break_references=True)
599+ with dbuser(config.IBranchDeleteJobSource.dbuser):
600+ merge_proposal1.target_branch.destroySelf(break_references=True)
601 self.assertRaises(SQLObjectNotFound,
602 BranchMergeProposal.get, merge_proposal1_id)
603
604 def test_deleteMergeProposalDependent(self):
605 """break_links enables deleting merge proposal dependant branches."""
606 merge_proposal1, merge_proposal2 = self.makeMergeProposals()
607- merge_proposal1.prerequisite_branch.destroySelf(break_references=True)
608+ with dbuser(config.IBranchDeleteJobSource.dbuser):
609+ merge_proposal1.prerequisite_branch.destroySelf(
610+ break_references=True)
611 self.assertEqual(None, merge_proposal1.prerequisite_branch)
612
613 def test_deleteSourceCodeReviewComment(self):
614@@ -1565,7 +1588,8 @@
615 comment = self.factory.makeCodeReviewComment()
616 comment_id = comment.id
617 branch = comment.branch_merge_proposal.source_branch
618- branch.destroySelf(break_references=True)
619+ with dbuser(config.IBranchDeleteJobSource.dbuser):
620+ branch.destroySelf(break_references=True)
621 self.assertRaises(
622 SQLObjectNotFound, CodeReviewComment.get, comment_id)
623
624@@ -1574,7 +1598,8 @@
625 comment = self.factory.makeCodeReviewComment()
626 comment_id = comment.id
627 branch = comment.branch_merge_proposal.target_branch
628- branch.destroySelf(break_references=True)
629+ with dbuser(config.IBranchDeleteJobSource.dbuser):
630+ branch.destroySelf(break_references=True)
631 self.assertRaises(
632 SQLObjectNotFound, CodeReviewComment.get, comment_id)
633
634@@ -1592,7 +1617,8 @@
635 bug1.linkBranch(self.branch, self.branch.owner)
636 bug_branch1 = bug1.linked_bugbranches[0]
637 bug_branch1_id = removeSecurityProxy(bug_branch1).id
638- self.branch.destroySelf(break_references=True)
639+ with dbuser(config.IBranchDeleteJobSource.dbuser):
640+ self.branch.destroySelf(break_references=True)
641 self.assertRaises(SQLObjectNotFound, BugBranch.get, bug_branch1_id)
642
643 def test_branchWithSpecRequirements(self):
644@@ -1612,7 +1638,8 @@
645 spec2 = self.factory.makeSpecification()
646 spec2.linkBranch(self.branch, self.branch.owner)
647 spec2_branch_id = self.branch.spec_links[1].id
648- self.branch.destroySelf(break_references=True)
649+ with dbuser(config.IBranchDeleteJobSource.dbuser):
650+ self.branch.destroySelf(break_references=True)
651 self.assertRaises(
652 SQLObjectNotFound, SpecificationBranch.get, spec1_branch_id)
653 self.assertRaises(
654@@ -1630,7 +1657,8 @@
655 """break_links allows deleting a series' branch."""
656 series1 = self.factory.makeProductSeries(branch=self.branch)
657 series2 = self.factory.makeProductSeries(branch=self.branch)
658- self.branch.destroySelf(break_references=True)
659+ with dbuser(config.IBranchDeleteJobSource.dbuser):
660+ self.branch.destroySelf(break_references=True)
661 self.assertEqual(None, series1.branch)
662 self.assertEqual(None, series2.branch)
663
664@@ -1661,7 +1689,8 @@
665 package.development_version.setBranch,
666 pocket, branch, package.distribution.owner)
667 self.assertEqual(False, branch.canBeDeleted())
668- branch.destroySelf(break_references=True)
669+ with dbuser(config.IBranchDeleteJobSource.dbuser):
670+ branch.destroySelf(break_references=True)
671 self.assertIs(None, package.getBranch(pocket))
672
673 def test_branchWithCodeImportRequirements(self):
674@@ -1676,7 +1705,8 @@
675 """break_references allows deleting a code import branch."""
676 code_import = self.factory.makeCodeImport()
677 code_import_id = code_import.id
678- code_import.branch.destroySelf(break_references=True)
679+ with dbuser(config.IBranchDeleteJobSource.dbuser):
680+ code_import.branch.destroySelf(break_references=True)
681 self.assertRaises(
682 NotFoundError, getUtility(ICodeImportSet).get, code_import_id)
683
684@@ -1685,14 +1715,16 @@
685 merge_proposal = self.factory.makeBranchMergeProposal()
686 merge_proposal.nominateReviewer(self.factory.makePerson(),
687 self.factory.makePerson())
688- merge_proposal.source_branch.destroySelf(break_references=True)
689+ with dbuser(config.IBranchDeleteJobSource.dbuser):
690+ merge_proposal.source_branch.destroySelf(break_references=True)
691
692 def test_targetBranchWithCodeReviewVoteReference(self):
693 """Break_references handles CodeReviewVoteReference target branch."""
694 merge_proposal = self.factory.makeBranchMergeProposal()
695 merge_proposal.nominateReviewer(self.factory.makePerson(),
696 self.factory.makePerson())
697- merge_proposal.target_branch.destroySelf(break_references=True)
698+ with dbuser(config.IBranchDeleteJobSource.dbuser):
699+ merge_proposal.target_branch.destroySelf(break_references=True)
700
701 def test_snap_requirements(self):
702 # If a branch is used by a snap package, the deletion requirements
703@@ -1706,7 +1738,8 @@
704 # break_references allows deleting a branch used by a snap package.
705 snap1 = self.factory.makeSnap(branch=self.branch)
706 snap2 = self.factory.makeSnap(branch=self.branch)
707- self.branch.destroySelf(break_references=True)
708+ with dbuser(config.IBranchDeleteJobSource.dbuser):
709+ self.branch.destroySelf(break_references=True)
710 transaction.commit()
711 self.assertIsNone(snap1.branch)
712 self.assertIsNone(snap2.branch)
713@@ -1715,7 +1748,8 @@
714 """ClearDependent.__call__ must clear the prerequisite branch."""
715 merge_proposal = removeSecurityProxy(self.makeMergeProposals()[0])
716 with person_logged_in(merge_proposal.prerequisite_branch.owner):
717- ClearDependentBranch(merge_proposal)()
718+ with dbuser(config.IBranchDeleteJobSource.dbuser):
719+ ClearDependentBranch(merge_proposal)()
720 self.assertEqual(None, merge_proposal.prerequisite_branch)
721
722 def test_ClearOfficialPackageBranch(self):
723@@ -1730,14 +1764,16 @@
724 pocket, branch, package.distribution.owner)
725 series_set = getUtility(IFindOfficialBranchLinks)
726 [link] = list(series_set.findForBranch(branch))
727- ClearOfficialPackageBranch(link)()
728+ with dbuser(config.IBranchDeleteJobSource.dbuser):
729+ ClearOfficialPackageBranch(link)()
730 self.assertIs(None, package.getBranch(pocket))
731
732 def test_ClearSeriesBranch(self):
733 """ClearSeriesBranch.__call__ must clear the user branch."""
734 series = removeSecurityProxy(self.factory.makeProductSeries(
735 branch=self.branch))
736- ClearSeriesBranch(series, self.branch)()
737+ with dbuser(config.IBranchDeleteJobSource.dbuser):
738+ ClearSeriesBranch(series, self.branch)()
739 self.assertEqual(None, series.branch)
740
741 def test_DeletionOperation(self):
742@@ -1749,7 +1785,8 @@
743 spec = self.factory.makeSpecification()
744 spec_link = spec.linkBranch(self.branch, self.branch.owner)
745 spec_link_id = spec_link.id
746- DeletionCallable(spec, 'blah', spec_link.destroySelf)()
747+ with dbuser(config.IBranchDeleteJobSource.dbuser):
748+ DeletionCallable(spec, 'blah', spec_link.destroySelf)()
749 self.assertRaises(SQLObjectNotFound, SpecificationBranch.get,
750 spec_link_id)
751
752@@ -1757,7 +1794,8 @@
753 """DeleteCodeImport.__call__ must delete the CodeImport."""
754 code_import = self.factory.makeCodeImport()
755 code_import_id = code_import.id
756- DeleteCodeImport(code_import)()
757+ with dbuser(config.IBranchDeleteJobSource.dbuser):
758+ DeleteCodeImport(code_import)()
759 self.assertRaises(
760 NotFoundError, getUtility(ICodeImportSet).get, code_import_id)
761
762
763=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
764--- lib/lp/code/model/tests/test_branchjob.py 2018-03-30 20:42:14 +0000
765+++ lib/lp/code/model/tests/test_branchjob.py 2019-03-21 15:55:38 +0000
766@@ -1,4 +1,4 @@
767-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
768+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
769 # GNU Affero General Public License version 3 (see the file LICENSE).
770
771 """Tests for BranchJobs."""
772@@ -16,7 +16,10 @@
773 from bzrlib.bzrdir import BzrDir
774 from bzrlib.revision import NULL_REVISION
775 from bzrlib.transport import get_transport
776-from fixtures import MockPatch
777+from fixtures import (
778+ FakeLogger,
779+ MockPatch,
780+ )
781 import pytz
782 from sqlobject import SQLObjectNotFound
783 from storm.locals import Store
784@@ -31,6 +34,7 @@
785 RepositoryFormat,
786 )
787 from lp.code.enums import (
788+ BranchDeletionStatus,
789 BranchMergeProposalStatus,
790 BranchSubscriptionDiffSize,
791 BranchSubscriptionNotificationLevel,
792@@ -38,6 +42,8 @@
793 )
794 from lp.code.errors import AlreadyLatestFormat
795 from lp.code.interfaces.branchjob import (
796+ IBranchDeleteJob,
797+ IBranchDeleteJobSource,
798 IBranchJob,
799 IBranchScanJob,
800 IBranchUpgradeJob,
801@@ -46,6 +52,7 @@
802 IRevisionMailJob,
803 IRosettaUploadJob,
804 )
805+from lp.code.interfaces.branchlookup import IBranchLookup
806 from lp.code.model.branchjob import (
807 BranchJob,
808 BranchJobDerived,
809@@ -72,6 +79,7 @@
810 from lp.services.job.model.job import Job
811 from lp.services.job.runner import JobRunner
812 from lp.services.job.tests import block_on_job
813+from lp.services.mail.sendmail import format_address_for_person
814 from lp.services.osutils import override_environ
815 from lp.services.webapp import canonical_url
816 from lp.testing import (
817@@ -86,6 +94,7 @@
818 CeleryBzrsyncdJobLayer,
819 DatabaseFunctionalLayer,
820 LaunchpadZopelessLayer,
821+ ZopelessDatabaseLayer,
822 )
823 from lp.testing.librarianhelpers import get_newest_librarian_file
824 from lp.testing.mail_helpers import pop_notifications
825@@ -1390,3 +1399,87 @@
826 os.makedirs(branch_path)
827 self.runReadyJobs()
828 self.assertFalse(os.path.exists(branch_path))
829+
830+
831+class TestBranchDeleteJob(TestCaseWithFactory):
832+
833+ layer = ZopelessDatabaseLayer
834+
835+ def test_providesInterface(self):
836+ branch = self.factory.makeAnyBranch()
837+ requester = branch.registrant
838+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
839+ self.assertProvides(job, IBranchDeleteJob)
840+
841+ def test_run(self):
842+ branch = self.factory.makeAnyBranch()
843+ branch_id = branch.id
844+ requester = branch.registrant
845+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
846+ removeSecurityProxy(branch).deletion_status = (
847+ BranchDeletionStatus.DELETING)
848+ logger = self.useFixture(FakeLogger())
849+ with dbuser(config.IBranchDeleteJobSource.dbuser):
850+ job.run()
851+ self.assertEqual('', logger.output)
852+ self.assertIsNone(getUtility(IBranchLookup).get(branch_id))
853+
854+ def test_already_deleted(self):
855+ branch = self.factory.makeAnyBranch()
856+ branch_name = branch.unique_name
857+ requester = branch.registrant
858+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
859+ branch.destroySelf()
860+ logger = self.useFixture(FakeLogger())
861+ with dbuser(config.IBranchDeleteJobSource.dbuser):
862+ job.run()
863+ self.assertEqual(
864+ 'Skipping branch %s because it has already been '
865+ 'deleted.\n' % branch_name,
866+ logger.output)
867+
868+ def test_not_deleting(self):
869+ # The job skips branches that aren't DELETING. This shouldn't be
870+ # possible in practice, but is a guard against accidents.
871+ branch = self.factory.makeAnyBranch()
872+ branch_id = branch.id
873+ branch_name = branch.unique_name
874+ self.assertNotEqual(
875+ BranchDeletionStatus.DELETING, branch.deletion_status)
876+ requester = branch.registrant
877+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
878+ logger = self.useFixture(FakeLogger())
879+ with dbuser(config.IBranchDeleteJobSource.dbuser):
880+ job.run()
881+ self.assertEqual(
882+ 'Skipping branch %s because its deletion status is not '
883+ 'DELETING.\n' % branch_name,
884+ logger.output)
885+ self.assertEqual(branch, getUtility(IBranchLookup).get(branch_id))
886+
887+ def test_error(self):
888+ # If deleting the branch fails, an error message is sent to the
889+ # requester and the deletion status is set back to ACTIVE. Most
890+ # cases where this can happen are caught earlier, but this can still
891+ # happen if a branch stacked on the to-be-deleted branch is created
892+ # between the deletion job being created and being run.
893+ branch = self.factory.makeAnyBranch()
894+ branch_id = branch.id
895+ requester = branch.registrant
896+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
897+ removeSecurityProxy(branch).deletion_status = (
898+ BranchDeletionStatus.DELETING)
899+ self.factory.makeAnyBranch(stacked_on=branch)
900+ logger = self.useFixture(FakeLogger())
901+ with dbuser(config.IBranchDeleteJobSource.dbuser):
902+ JobRunner([job]).runJobHandleError(job)
903+ self.assertIn(
904+ 'failed with user error CannotDeleteBranch', logger.output)
905+ self.assertEqual(branch, getUtility(IBranchLookup).get(branch_id))
906+ self.assertEqual(BranchDeletionStatus.ACTIVE, branch.deletion_status)
907+ self.assertEqual([], self.oopses)
908+ [mail] = pop_notifications()
909+ self.assertEqual(format_address_for_person(requester), mail['to'])
910+ self.assertEqual(
911+ 'Launchpad error while deleting a branch', mail['subject'])
912+ self.assertIn('Cannot delete branch', mail.get_payload(decode=True))
913
914=== modified file 'lib/lp/code/stories/webservice/xx-branch.txt'
915--- lib/lp/code/stories/webservice/xx-branch.txt 2018-05-13 10:35:52 +0000
916+++ lib/lp/code/stories/webservice/xx-branch.txt 2019-03-21 15:55:38 +0000
917@@ -112,6 +112,7 @@
918 control_format: None
919 date_created: u'2009-01-01T00:00:00+00:00'
920 date_last_modified: u'2009-01-01T00:00:00+00:00'
921+ deletion_status: u'Active'
922 dependent_branches_collection_link: u'.../~eric/fooix/trunk/dependent_branches'
923 description: None
924 display_name: u'lp://dev/~eric/fooix/trunk'
925
926=== modified file 'lib/lp/services/config/schema-lazr.conf'
927--- lib/lp/services/config/schema-lazr.conf 2019-03-18 12:22:55 +0000
928+++ lib/lp/services/config/schema-lazr.conf 2019-03-21 15:55:38 +0000
929@@ -1867,6 +1867,7 @@
930 # dbuser, the crontab_group, and the module that the job source class
931 # can be loaded from.
932 job_sources:
933+ IBranchDeleteJobSource,
934 IBranchModifiedMailJobSource,
935 ICommercialExpiredJobSource,
936 IExpiringMembershipNotificationJobSource,
937@@ -1888,6 +1889,11 @@
938 ITeamJoinNotificationJobSource,
939 IThirtyDayCommercialExpirationJobSource
940
941+[IBranchDeleteJobSource]
942+module: lp.code.interfaces.branchjob
943+dbuser: branch-delete-job
944+crontab_group: MAIN
945+
946 [IBranchMergeProposalJobSource]
947 module: lp.code.interfaces.branchmergeproposal
948 dbuser: merge-proposal-jobs