launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23438
[Merge] lp:~cjwatson/launchpad/branch-delete-job into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/branch-delete-job into lp:launchpad with lp:~cjwatson/launchpad/code-delete-no-recipe-preload as a prerequisite.
Commit message:
Add a branch deletion job.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1793266 in Launchpad itself: "Unable to delete repository"
https://bugs.launchpad.net/launchpad/+bug/1793266
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/branch-delete-job/+merge/364907
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/branch-delete-job into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2019-03-11 14:51:16 +0000
+++ database/schema/security.cfg 2019-03-21 15:55:38 +0000
@@ -2593,3 +2593,51 @@
public.teamparticipation = SELECT
public.webhook = SELECT
public.webhookjob = SELECT, INSERT
+
+[branch-delete-job]
+type=user
+groups=script
+public.accessartifact = SELECT, DELETE
+public.accessartifactgrant = SELECT, DELETE
+public.accesspolicyartifact = SELECT, DELETE
+public.archive = SELECT
+public.branch = SELECT, UPDATE, DELETE
+public.branchjob = SELECT, INSERT, DELETE
+public.branchmergeproposal = SELECT, UPDATE, DELETE
+public.branchmergeproposaljob = SELECT, DELETE
+public.branchsubscription = SELECT, DELETE
+public.bug = SELECT
+public.bugbranch = SELECT, DELETE
+public.bugtask = SELECT
+public.buildfarmjob = SELECT, DELETE
+public.buildqueue = SELECT, DELETE
+public.codeimport = SELECT, DELETE
+public.codeimportjob = SELECT, DELETE
+public.codereviewinlinecomment = SELECT, DELETE
+public.codereviewinlinecommentdraft = SELECT, DELETE
+public.codereviewmessage = SELECT, DELETE
+public.codereviewvote = SELECT, DELETE
+public.distribution = SELECT
+public.distributionsourcepackage = SELECT, DELETE
+public.distroseries = SELECT
+public.emailaddress = SELECT
+public.job = SELECT, INSERT, UPDATE, DELETE
+public.person = SELECT
+public.previewdiff = SELECT, DELETE
+public.product = SELECT
+public.productseries = SELECT, UPDATE
+public.seriessourcepackagebranch = SELECT, DELETE
+public.snap = SELECT, UPDATE
+public.sourcepackage = SELECT
+public.sourcepackagename = SELECT
+public.sourcepackagepublishinghistory = SELECT
+public.sourcepackagerecipe = SELECT, DELETE
+public.sourcepackagerecipebuild = SELECT, UPDATE
+public.sourcepackagerecipedata = SELECT, DELETE
+public.sourcepackagerecipedatainstruction = SELECT, DELETE
+public.sourcepackagerecipedistroseries = SELECT, DELETE
+public.sourcepackagerelease = SELECT
+public.specificationbranch = SELECT, DELETE
+public.translationtemplatesbuild = SELECT, DELETE
+public.webhook = SELECT, DELETE
+public.webhookjob = SELECT, DELETE
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2018-10-16 15:52:00 +0000
+++ lib/lp/code/configure.zcml 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -602,6 +602,10 @@
<allow interface="lp.code.interfaces.branchjob.IBranchModifiedMailJob"/>
<allow interface="lp.code.interfaces.branchjob.IBranchJob"/>
</class>
+ <class class="lp.code.model.branchjob.BranchDeleteJob">
+ <allow interface="lp.code.interfaces.branchjob.IBranchDeleteJob"/>
+ <allow interface="lp.code.interfaces.branchjob.IBranchJob"/>
+ </class>
<securedutility
component="lp.code.model.branchjob.RevisionMailJob"
provides="lp.code.interfaces.branchjob.IRevisionMailJobSource">
@@ -627,6 +631,11 @@
provides="lp.code.interfaces.branchjob.IBranchModifiedMailJobSource">
<allow interface="lp.code.interfaces.branchjob.IBranchModifiedMailJobSource"/>
</securedutility>
+ <securedutility
+ component="lp.code.model.branchjob.BranchDeleteJob"
+ provides="lp.code.interfaces.branchjob.IBranchDeleteJobSource">
+ <allow interface="lp.code.interfaces.branchjob.IBranchDeleteJobSource"/>
+ </securedutility>
<!-- CodeImport -->
=== modified file 'lib/lp/code/enums.py'
--- lib/lp/code/enums.py 2018-10-19 23:10:41 +0000
+++ lib/lp/code/enums.py 2019-03-21 15:55:38 +0000
@@ -1,10 +1,11 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Enumerations used in the lp/code modules."""
__metaclass__ = type
__all__ = [
+ 'BranchDeletionStatus',
'BranchLifecycleStatus',
'BranchLifecycleStatusFilter',
'BranchListingSort',
@@ -116,6 +117,16 @@
""")
+class BranchDeletionStatus(DBEnumeratedType):
+ """Branch Deletion Status
+
+ The deletion status of a branch is used to track asynchronous deletion.
+ """
+
+ ACTIVE = DBItem(0, "Active")
+ DELETING = DBItem(1, "Deleting")
+
+
class GitRepositoryType(DBEnumeratedType):
"""Git Repository Type
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2019-01-23 17:13:48 +0000
+++ lib/lp/code/interfaces/branch.py 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Branch interfaces."""
@@ -75,6 +75,7 @@
RepositoryFormat,
)
from lp.code.enums import (
+ BranchDeletionStatus,
BranchLifecycleStatus,
BranchMergeProposalStatus,
BranchSubscriptionDiffSize,
@@ -293,6 +294,11 @@
accepted).
"""
+ deletion_status = exported(Choice(
+ title=_("Deletion status"), required=True, readonly=True,
+ vocabulary=BranchDeletionStatus,
+ description=_("The deletion status of this branch.")))
+
# People attributes
registrant = exported(
PublicPersonChoice(
=== modified file 'lib/lp/code/interfaces/branchjob.py'
--- lib/lp/code/interfaces/branchjob.py 2015-09-01 17:10:46 +0000
+++ lib/lp/code/interfaces/branchjob.py 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""BranchJob interfaces."""
@@ -8,6 +8,8 @@
__all__ = [
+ 'IBranchDeleteJob',
+ 'IBranchDeleteJobSource',
'IBranchJob',
'IBranchModifiedMailJob',
'IBranchModifiedMailJobSource',
@@ -201,3 +203,19 @@
:param user: The `IPerson` who modified the branch.
:param branch_delta: An `IBranchDelta` describing the changes.
"""
+
+
+class IBranchDeleteJob(IRunnableJob):
+ """A Job that deletes a branch from the database."""
+
+ branch_id = Int(title=_('The id of the branch to delete.'))
+
+
+class IBranchDeleteJobSource(IJobSource):
+
+ def create(branch, requester):
+ """Delete a branch from the database.
+
+ :param branch: The `IBranch` to delete.
+ :param requester: The `IPerson` who requested the deletion.
+ """
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2019-03-21 15:55:38 +0000
+++ lib/lp/code/model/branch.py 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -82,6 +82,7 @@
RepositoryFormat,
)
from lp.code.enums import (
+ BranchDeletionStatus,
BranchLifecycleStatus,
BranchMergeProposalStatus,
BranchType,
@@ -304,6 +305,23 @@
# such subscriptions.
getUtility(IRemoveArtifactSubscriptionsJobSource).create(who, [self])
+ _deletion_status = EnumCol(
+ dbName='deletion_status', enum=BranchDeletionStatus,
+ default=BranchDeletionStatus.ACTIVE)
+
+ @property
+ def deletion_status(self):
+ # XXX cjwatson 2019-03-19: Remove once this column has been
+ # backfilled.
+ if self._deletion_status is None:
+ return BranchDeletionStatus.ACTIVE
+ else:
+ return self._deletion_status
+
+ @deletion_status.setter
+ def deletion_status(self, value):
+ self._deletion_status = value
+
registrant = ForeignKey(
dbName='registrant', foreignKey='Person',
storm_validator=validate_public_person, notNull=True)
=== modified file 'lib/lp/code/model/branchjob.py'
--- lib/lp/code/model/branchjob.py 2018-03-30 20:42:14 +0000
+++ lib/lp/code/model/branchjob.py 2019-03-21 15:55:38 +0000
@@ -1,7 +1,8 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__all__ = [
+ 'BranchDeleteJob',
'BranchJob',
'BranchModifiedMailJob',
'BranchScanJob',
@@ -56,17 +57,22 @@
implementer,
provider,
)
+from zope.security.proxy import removeSecurityProxy
from lp.code.bzr import (
branch_revision_history,
get_branch_formats,
)
from lp.code.enums import (
+ BranchDeletionStatus,
BranchMergeProposalStatus,
BranchSubscriptionDiffSize,
BranchSubscriptionNotificationLevel,
)
+from lp.code.errors import CannotDeleteBranch
from lp.code.interfaces.branchjob import (
+ IBranchDeleteJob,
+ IBranchDeleteJobSource,
IBranchJob,
IBranchModifiedMailJob,
IBranchModifiedMailJobSource,
@@ -82,6 +88,7 @@
IRosettaUploadJob,
IRosettaUploadJobSource,
)
+from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.mail.branch import BranchMailer
from lp.code.model.branch import Branch
from lp.code.model.branchmergeproposal import BranchMergeProposal
@@ -121,6 +128,7 @@
BaseRunnableJobSource,
)
from lp.services.mail.sendmail import format_address_for_person
+from lp.services.scripts import log
from lp.services.utils import text_delta
from lp.services.webapp import canonical_url
from lp.translations.interfaces.translationimportqueue import (
@@ -195,6 +203,12 @@
This job runs against a branch to send emails about modifications.
""")
+ DELETE_BRANCH = DBItem(9, """
+ Delete branch
+
+ This job deletes a branch from the database.
+ """)
+
@implementer(IBranchJob)
class BranchJob(SQLBase):
@@ -295,7 +309,9 @@
vars.extend([
('branch_job_id', self.context.id),
('branch_job_type', self.context.job_type.title)])
- if self.context.branch is not None:
+ if 'branch_name' in self.metadata:
+ vars.append(('branch_name', self.metadata['branch_name']))
+ elif self.context.branch is not None:
vars.append(('branch_name', self.context.branch.unique_name))
return vars
@@ -347,7 +363,6 @@
def run(self):
"""See `IBranchScanJob`."""
- from lp.services.scripts import log
with server(get_ro_server(), no_replace=True):
try:
with try_advisory_lock(
@@ -1056,3 +1071,69 @@
def run(self):
"""See `IBranchModifiedMailJob`."""
self.getMailer().sendAll()
+
+
+@implementer(IBranchDeleteJob)
+@provider(IBranchDeleteJobSource)
+class BranchDeleteJob(BranchJobDerived):
+ """A Job that deletes a branch from the database."""
+
+ class_job_type = BranchJobType.DELETE_BRANCH
+
+ user_error_types = (CannotDeleteBranch,)
+
+ config = config.IBranchDeleteJobSource
+
+ def __repr__(self):
+ return '<DELETE_BRANCH branch job (%(id)s) for %(branch)s>' % {
+ 'id': self.context.id,
+ 'branch': self.branch_name,
+ }
+
+ def getOperationDescription(self):
+ return 'deleting a branch'
+
+ @classmethod
+ def create(cls, branch, requester):
+ """See `IBranchDeleteJobSource`."""
+ metadata = {'branch_id': branch.id, 'branch_name': branch.unique_name}
+ # The BranchJob has a branch of None, because we don't want to
+ # delete this job while trying to delete the branch.
+ branch_job = BranchJob(
+ None, cls.class_job_type, metadata, requester=requester)
+ job = cls(branch_job)
+ job.celeryRunOnCommit()
+ return job
+
+ @property
+ def branch_id(self):
+ return self.metadata['branch_id']
+
+ @property
+ def branch_name(self):
+ return self.metadata['branch_name']
+
+ def run(self):
+ """See `IBranchDeleteJob`."""
+ branch = getUtility(IBranchLookup).get(self.branch_id)
+ if branch is None:
+ log.info(
+ 'Skipping branch %s because it has already been deleted.' %
+ self.branch_name)
+ elif branch.deletion_status != BranchDeletionStatus.DELETING:
+ log.warning(
+ 'Skipping branch %s because its deletion status is not '
+ 'DELETING.' % self.branch_name)
+ else:
+ try:
+ branch.destroySelf(break_references=True)
+ except CannotDeleteBranch:
+ # Set the deletion status back to ACTIVE so that it's
+ # possible to try again. We don't attempt to undo the
+ # renaming at the moment. Do this in its own transaction
+ # since the job runner will abort the transaction.
+ transaction.abort()
+ removeSecurityProxy(branch).deletion_status = (
+ BranchDeletionStatus.ACTIVE)
+ transaction.commit()
+ raise
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2019-01-28 18:09:21 +0000
+++ lib/lp/code/model/tests/test_branch.py 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Branches."""
@@ -1247,7 +1247,8 @@
"deleted.")
branch_id = self.branch.id
branch_set = getUtility(IBranchLookup)
- self.branch.destroySelf()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf()
self.assertIsNone(
branch_set.get(branch_id), "The branch has not been deleted.")
@@ -1280,7 +1281,8 @@
bug.linkBranch(self.branch, self.user)
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch linked to a bug is not deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_specBranchLinkDisablesDeletion(self):
"""A branch linked to a spec cannot be deleted."""
@@ -1291,7 +1293,8 @@
spec.linkBranch(self.branch, self.user)
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch linked to a spec is not deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_associatedProductSeriesBranchDisablesDeletion(self):
"""A branch linked as a branch to a product series cannot be
@@ -1301,14 +1304,16 @@
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch that is a user branch for a product series"
" is not deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_productSeriesTranslationsBranchDisablesDeletion(self):
self.product.development_focus.translations_branch = self.branch
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch that is a translations branch for a "
"product series is not deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_revisionsDeletable(self):
"""A branch that has some revisions can be deleted."""
@@ -1321,7 +1326,8 @@
self.assertEqual(self.branch.canBeDeleted(), True,
"A branch that has a revision is deletable.")
unique_name = self.branch.unique_name
- self.branch.destroySelf()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf()
# Commit again to trigger the deferred indices.
transaction.commit()
branch_lookup = getUtility(IBranchLookup)
@@ -1335,7 +1341,8 @@
self.branch.addLandingTarget(self.user, target_branch)
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch with a landing target is not deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_landingCandidateDisablesDeletion(self):
"""A branch with a landing candidate cannot be deleted."""
@@ -1345,7 +1352,8 @@
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch with a landing candidate is not"
" deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_prerequisiteBranchDisablesDeletion(self):
"""A branch that is a prerequisite branch cannot be deleted."""
@@ -1357,14 +1365,16 @@
self.assertEqual(self.branch.canBeDeleted(), False,
"A branch with a prerequisite target is not "
"deletable.")
- self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.assertRaises(CannotDeleteBranch, self.branch.destroySelf)
def test_relatedBranchJobsDeleted(self):
# A branch with an associated branch job will delete those jobs.
branch = self.factory.makeBranch(
branch_format=BranchFormat.BZR_BRANCH_6)
removeSecurityProxy(branch).requestUpgrade(branch.owner)
- branch.destroySelf()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch.destroySelf()
# Need to commit the transaction to fire off the constraint checks.
transaction.commit()
@@ -1373,7 +1383,8 @@
# should be cleared.
dev_focus = self.branch.product.development_focus
dev_focus.translations_branch = self.branch
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
def test_related_TranslationTemplatesBuild_cleaned_out(self):
# A TranslationTemplatesBuild may come with a BuildQueue entry.
@@ -1381,7 +1392,8 @@
# remove the TTB.
build = self.factory.makeTranslationTemplatesBuild()
build.queueBuild()
- build.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ build.branch.destroySelf(break_references=True)
def test_unrelated_TranslationTemplatesBuild_intact(self):
# No innocent BuildQueue entries are harmed in deleting a
@@ -1391,7 +1403,8 @@
other_build = self.factory.makeTranslationTemplatesBuild()
other_bq = other_build.queueBuild()
- build.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ build.branch.destroySelf(break_references=True)
store = Store.of(build)
# The BuildQueue for the job whose branch we deleted is gone.
@@ -1406,7 +1419,8 @@
branch = self.factory.makeAnyBranch()
branch_id = branch.id
store = Store.of(branch)
- branch.destroySelf()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch.destroySelf()
jobs = store.find(
BranchJob,
BranchJob.job_type == BranchJobType.RECLAIM_BRANCH_SPACE)
@@ -1417,7 +1431,8 @@
def test_destroySelf_with_SourcePackageRecipe(self):
"""If branch is a base_branch in a recipe, it is deleted."""
recipe = self.factory.makeSourcePackageRecipe()
- recipe.base_branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ recipe.base_branch.destroySelf(break_references=True)
def test_destroySelf_with_SourcePackageRecipe_as_non_base(self):
"""If branch is referred to by a recipe, it is deleted."""
@@ -1425,7 +1440,8 @@
branch2 = self.factory.makeAnyBranch()
self.factory.makeSourcePackageRecipe(
branches=[branch1, branch2])
- branch2.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch2.destroySelf(break_references=True)
def test_destroySelf_with_inline_comments_draft(self):
# Draft inline comments related to a deleted branch (source
@@ -1439,7 +1455,8 @@
previewdiff_id=preview_diff.id,
person=self.user,
comments={'1': 'Should vanish.'})
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
def test_destroySelf_with_inline_comments_published(self):
# Published inline comments related to a deleted branch (source
@@ -1455,12 +1472,14 @@
previewdiff_id=preview_diff.id,
inline_comments={'1': 'Must disappear.'},
)
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
def test_related_webhooks_deleted(self):
webhook = self.factory.makeWebhook(target=self.branch)
webhook.ping()
- self.branch.destroySelf()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf()
transaction.commit()
self.assertRaises(LostObjectError, getattr, webhook, 'target')
@@ -1541,7 +1560,8 @@
merge_proposal1, merge_proposal2 = self.makeMergeProposals()
merge_proposal1_id = merge_proposal1.id
BranchMergeProposal.get(merge_proposal1_id)
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
self.assertRaises(SQLObjectNotFound,
BranchMergeProposal.get, merge_proposal1_id)
@@ -1550,14 +1570,17 @@
merge_proposal1, merge_proposal2 = self.makeMergeProposals()
merge_proposal1_id = merge_proposal1.id
BranchMergeProposal.get(merge_proposal1_id)
- merge_proposal1.target_branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ merge_proposal1.target_branch.destroySelf(break_references=True)
self.assertRaises(SQLObjectNotFound,
BranchMergeProposal.get, merge_proposal1_id)
def test_deleteMergeProposalDependent(self):
"""break_links enables deleting merge proposal dependant branches."""
merge_proposal1, merge_proposal2 = self.makeMergeProposals()
- merge_proposal1.prerequisite_branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ merge_proposal1.prerequisite_branch.destroySelf(
+ break_references=True)
self.assertEqual(None, merge_proposal1.prerequisite_branch)
def test_deleteSourceCodeReviewComment(self):
@@ -1565,7 +1588,8 @@
comment = self.factory.makeCodeReviewComment()
comment_id = comment.id
branch = comment.branch_merge_proposal.source_branch
- branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch.destroySelf(break_references=True)
self.assertRaises(
SQLObjectNotFound, CodeReviewComment.get, comment_id)
@@ -1574,7 +1598,8 @@
comment = self.factory.makeCodeReviewComment()
comment_id = comment.id
branch = comment.branch_merge_proposal.target_branch
- branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch.destroySelf(break_references=True)
self.assertRaises(
SQLObjectNotFound, CodeReviewComment.get, comment_id)
@@ -1592,7 +1617,8 @@
bug1.linkBranch(self.branch, self.branch.owner)
bug_branch1 = bug1.linked_bugbranches[0]
bug_branch1_id = removeSecurityProxy(bug_branch1).id
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
self.assertRaises(SQLObjectNotFound, BugBranch.get, bug_branch1_id)
def test_branchWithSpecRequirements(self):
@@ -1612,7 +1638,8 @@
spec2 = self.factory.makeSpecification()
spec2.linkBranch(self.branch, self.branch.owner)
spec2_branch_id = self.branch.spec_links[1].id
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
self.assertRaises(
SQLObjectNotFound, SpecificationBranch.get, spec1_branch_id)
self.assertRaises(
@@ -1630,7 +1657,8 @@
"""break_links allows deleting a series' branch."""
series1 = self.factory.makeProductSeries(branch=self.branch)
series2 = self.factory.makeProductSeries(branch=self.branch)
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
self.assertEqual(None, series1.branch)
self.assertEqual(None, series2.branch)
@@ -1661,7 +1689,8 @@
package.development_version.setBranch,
pocket, branch, package.distribution.owner)
self.assertEqual(False, branch.canBeDeleted())
- branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ branch.destroySelf(break_references=True)
self.assertIs(None, package.getBranch(pocket))
def test_branchWithCodeImportRequirements(self):
@@ -1676,7 +1705,8 @@
"""break_references allows deleting a code import branch."""
code_import = self.factory.makeCodeImport()
code_import_id = code_import.id
- code_import.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ code_import.branch.destroySelf(break_references=True)
self.assertRaises(
NotFoundError, getUtility(ICodeImportSet).get, code_import_id)
@@ -1685,14 +1715,16 @@
merge_proposal = self.factory.makeBranchMergeProposal()
merge_proposal.nominateReviewer(self.factory.makePerson(),
self.factory.makePerson())
- merge_proposal.source_branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ merge_proposal.source_branch.destroySelf(break_references=True)
def test_targetBranchWithCodeReviewVoteReference(self):
"""Break_references handles CodeReviewVoteReference target branch."""
merge_proposal = self.factory.makeBranchMergeProposal()
merge_proposal.nominateReviewer(self.factory.makePerson(),
self.factory.makePerson())
- merge_proposal.target_branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ merge_proposal.target_branch.destroySelf(break_references=True)
def test_snap_requirements(self):
# If a branch is used by a snap package, the deletion requirements
@@ -1706,7 +1738,8 @@
# break_references allows deleting a branch used by a snap package.
snap1 = self.factory.makeSnap(branch=self.branch)
snap2 = self.factory.makeSnap(branch=self.branch)
- self.branch.destroySelf(break_references=True)
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ self.branch.destroySelf(break_references=True)
transaction.commit()
self.assertIsNone(snap1.branch)
self.assertIsNone(snap2.branch)
@@ -1715,7 +1748,8 @@
"""ClearDependent.__call__ must clear the prerequisite branch."""
merge_proposal = removeSecurityProxy(self.makeMergeProposals()[0])
with person_logged_in(merge_proposal.prerequisite_branch.owner):
- ClearDependentBranch(merge_proposal)()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ ClearDependentBranch(merge_proposal)()
self.assertEqual(None, merge_proposal.prerequisite_branch)
def test_ClearOfficialPackageBranch(self):
@@ -1730,14 +1764,16 @@
pocket, branch, package.distribution.owner)
series_set = getUtility(IFindOfficialBranchLinks)
[link] = list(series_set.findForBranch(branch))
- ClearOfficialPackageBranch(link)()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ ClearOfficialPackageBranch(link)()
self.assertIs(None, package.getBranch(pocket))
def test_ClearSeriesBranch(self):
"""ClearSeriesBranch.__call__ must clear the user branch."""
series = removeSecurityProxy(self.factory.makeProductSeries(
branch=self.branch))
- ClearSeriesBranch(series, self.branch)()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ ClearSeriesBranch(series, self.branch)()
self.assertEqual(None, series.branch)
def test_DeletionOperation(self):
@@ -1749,7 +1785,8 @@
spec = self.factory.makeSpecification()
spec_link = spec.linkBranch(self.branch, self.branch.owner)
spec_link_id = spec_link.id
- DeletionCallable(spec, 'blah', spec_link.destroySelf)()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ DeletionCallable(spec, 'blah', spec_link.destroySelf)()
self.assertRaises(SQLObjectNotFound, SpecificationBranch.get,
spec_link_id)
@@ -1757,7 +1794,8 @@
"""DeleteCodeImport.__call__ must delete the CodeImport."""
code_import = self.factory.makeCodeImport()
code_import_id = code_import.id
- DeleteCodeImport(code_import)()
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ DeleteCodeImport(code_import)()
self.assertRaises(
NotFoundError, getUtility(ICodeImportSet).get, code_import_id)
=== modified file 'lib/lp/code/model/tests/test_branchjob.py'
--- lib/lp/code/model/tests/test_branchjob.py 2018-03-30 20:42:14 +0000
+++ lib/lp/code/model/tests/test_branchjob.py 2019-03-21 15:55:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for BranchJobs."""
@@ -16,7 +16,10 @@
from bzrlib.bzrdir import BzrDir
from bzrlib.revision import NULL_REVISION
from bzrlib.transport import get_transport
-from fixtures import MockPatch
+from fixtures import (
+ FakeLogger,
+ MockPatch,
+ )
import pytz
from sqlobject import SQLObjectNotFound
from storm.locals import Store
@@ -31,6 +34,7 @@
RepositoryFormat,
)
from lp.code.enums import (
+ BranchDeletionStatus,
BranchMergeProposalStatus,
BranchSubscriptionDiffSize,
BranchSubscriptionNotificationLevel,
@@ -38,6 +42,8 @@
)
from lp.code.errors import AlreadyLatestFormat
from lp.code.interfaces.branchjob import (
+ IBranchDeleteJob,
+ IBranchDeleteJobSource,
IBranchJob,
IBranchScanJob,
IBranchUpgradeJob,
@@ -46,6 +52,7 @@
IRevisionMailJob,
IRosettaUploadJob,
)
+from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.model.branchjob import (
BranchJob,
BranchJobDerived,
@@ -72,6 +79,7 @@
from lp.services.job.model.job import Job
from lp.services.job.runner import JobRunner
from lp.services.job.tests import block_on_job
+from lp.services.mail.sendmail import format_address_for_person
from lp.services.osutils import override_environ
from lp.services.webapp import canonical_url
from lp.testing import (
@@ -86,6 +94,7 @@
CeleryBzrsyncdJobLayer,
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
+ ZopelessDatabaseLayer,
)
from lp.testing.librarianhelpers import get_newest_librarian_file
from lp.testing.mail_helpers import pop_notifications
@@ -1390,3 +1399,87 @@
os.makedirs(branch_path)
self.runReadyJobs()
self.assertFalse(os.path.exists(branch_path))
+
+
+class TestBranchDeleteJob(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+
+ def test_providesInterface(self):
+ branch = self.factory.makeAnyBranch()
+ requester = branch.registrant
+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
+ self.assertProvides(job, IBranchDeleteJob)
+
+ def test_run(self):
+ branch = self.factory.makeAnyBranch()
+ branch_id = branch.id
+ requester = branch.registrant
+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
+ removeSecurityProxy(branch).deletion_status = (
+ BranchDeletionStatus.DELETING)
+ logger = self.useFixture(FakeLogger())
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ job.run()
+ self.assertEqual('', logger.output)
+ self.assertIsNone(getUtility(IBranchLookup).get(branch_id))
+
+ def test_already_deleted(self):
+ branch = self.factory.makeAnyBranch()
+ branch_name = branch.unique_name
+ requester = branch.registrant
+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
+ branch.destroySelf()
+ logger = self.useFixture(FakeLogger())
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ job.run()
+ self.assertEqual(
+ 'Skipping branch %s because it has already been '
+ 'deleted.\n' % branch_name,
+ logger.output)
+
+ def test_not_deleting(self):
+ # The job skips branches that aren't DELETING. This shouldn't be
+ # possible in practice, but is a guard against accidents.
+ branch = self.factory.makeAnyBranch()
+ branch_id = branch.id
+ branch_name = branch.unique_name
+ self.assertNotEqual(
+ BranchDeletionStatus.DELETING, branch.deletion_status)
+ requester = branch.registrant
+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
+ logger = self.useFixture(FakeLogger())
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ job.run()
+ self.assertEqual(
+ 'Skipping branch %s because its deletion status is not '
+ 'DELETING.\n' % branch_name,
+ logger.output)
+ self.assertEqual(branch, getUtility(IBranchLookup).get(branch_id))
+
+ def test_error(self):
+ # If deleting the branch fails, an error message is sent to the
+ # requester and the deletion status is set back to ACTIVE. Most
+ # cases where this can happen are caught earlier, but this can still
+ # happen if a branch stacked on the to-be-deleted branch is created
+ # between the deletion job being created and being run.
+ branch = self.factory.makeAnyBranch()
+ branch_id = branch.id
+ requester = branch.registrant
+ job = getUtility(IBranchDeleteJobSource).create(branch, requester)
+ removeSecurityProxy(branch).deletion_status = (
+ BranchDeletionStatus.DELETING)
+ self.factory.makeAnyBranch(stacked_on=branch)
+ logger = self.useFixture(FakeLogger())
+ with dbuser(config.IBranchDeleteJobSource.dbuser):
+ JobRunner([job]).runJobHandleError(job)
+ self.assertIn(
+ 'failed with user error CannotDeleteBranch', logger.output)
+ self.assertEqual(branch, getUtility(IBranchLookup).get(branch_id))
+ self.assertEqual(BranchDeletionStatus.ACTIVE, branch.deletion_status)
+ self.assertEqual([], self.oopses)
+ [mail] = pop_notifications()
+ self.assertEqual(format_address_for_person(requester), mail['to'])
+ self.assertEqual(
+ 'Launchpad error while deleting a branch', mail['subject'])
+ self.assertIn('Cannot delete branch', mail.get_payload(decode=True))
=== modified file 'lib/lp/code/stories/webservice/xx-branch.txt'
--- lib/lp/code/stories/webservice/xx-branch.txt 2018-05-13 10:35:52 +0000
+++ lib/lp/code/stories/webservice/xx-branch.txt 2019-03-21 15:55:38 +0000
@@ -112,6 +112,7 @@
control_format: None
date_created: u'2009-01-01T00:00:00+00:00'
date_last_modified: u'2009-01-01T00:00:00+00:00'
+ deletion_status: u'Active'
dependent_branches_collection_link: u'.../~eric/fooix/trunk/dependent_branches'
description: None
display_name: u'lp://dev/~eric/fooix/trunk'
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2019-03-18 12:22:55 +0000
+++ lib/lp/services/config/schema-lazr.conf 2019-03-21 15:55:38 +0000
@@ -1867,6 +1867,7 @@
# dbuser, the crontab_group, and the module that the job source class
# can be loaded from.
job_sources:
+ IBranchDeleteJobSource,
IBranchModifiedMailJobSource,
ICommercialExpiredJobSource,
IExpiringMembershipNotificationJobSource,
@@ -1888,6 +1889,11 @@
ITeamJoinNotificationJobSource,
IThirtyDayCommercialExpirationJobSource
+[IBranchDeleteJobSource]
+module: lp.code.interfaces.branchjob
+dbuser: branch-delete-job
+crontab_group: MAIN
+
[IBranchMergeProposalJobSource]
module: lp.code.interfaces.branchmergeproposal
dbuser: merge-proposal-jobs
Follow ups
