launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18595
[Merge] lp:~cjwatson/launchpad/git-repository-delete into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-delete into lp:launchpad.
Commit message:
Implement the model/webservice side of Git repository deletion.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1456583 in Launchpad itself: "Can't delete Git repositories"
https://bugs.launchpad.net/launchpad/+bug/1456583
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-delete/+merge/259487
Implement the model/webservice side of Git repository deletion.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-delete into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2015-05-19 00:07:44 +0000
+++ database/schema/security.cfg 2015-05-19 11:33:34 +0000
@@ -2039,6 +2039,7 @@
[reclaim-branch-space]
groups=script
public.branchjob = SELECT
+public.gitjob = SELECT
public.job = SELECT, UPDATE
type=user
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2015-04-22 14:52:10 +0000
+++ lib/lp/code/configure.zcml 2015-05-19 11:33:34 +0000
@@ -204,7 +204,9 @@
permission="launchpad.Edit"
interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalEdit"
set_attributes="description whiteboard merged_revno commit_message
- root_message_id prerequisite_branch"/>
+ root_message_id prerequisite_branch
+ prerequisite_git_repository prerequisite_git_path
+ prerequisite_git_commit_sha1"/>
<require
permission="launchpad.AnyAllowedPerson"
interface="lp.code.interfaces.branchmergeproposal.IBranchMergeProposalAnyAllowedPerson"/>
@@ -941,10 +943,19 @@
provides="lp.code.interfaces.gitjob.IGitRefScanJobSource">
<allow interface="lp.code.interfaces.gitjob.IGitRefScanJobSource" />
</securedutility>
+ <securedutility
+ component="lp.code.model.gitjob.ReclaimGitRepositorySpaceJob"
+ provides="lp.code.interfaces.gitjob.IReclaimGitRepositorySpaceJobSource">
+ <allow interface="lp.code.interfaces.gitjob.IReclaimGitRepositorySpaceJobSource" />
+ </securedutility>
<class class="lp.code.model.gitjob.GitRefScanJob">
<allow interface="lp.code.interfaces.gitjob.IGitJob" />
<allow interface="lp.code.interfaces.gitjob.IGitRefScanJob" />
</class>
+ <class class="lp.code.model.gitjob.ReclaimGitRepositorySpaceJob">
+ <allow interface="lp.code.interfaces.gitjob.IGitJob" />
+ <allow interface="lp.code.interfaces.gitjob.IReclaimGitRepositorySpaceJob" />
+ </class>
<lp:help-folder folder="help" name="+help-code" />
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/errors.py 2015-05-19 11:33:34 +0000
@@ -20,6 +20,7 @@
'BuildNotAllowedForDistro',
'BranchMergeProposalExists',
'CannotDeleteBranch',
+ 'CannotDeleteGitRepository',
'CannotHaveLinkedBranch',
'CannotUpgradeBranch',
'CannotUpgradeNonHosted',
@@ -34,6 +35,7 @@
'GitRepositoryCreationForbidden',
'GitRepositoryCreatorNotMemberOfOwnerTeam',
'GitRepositoryCreatorNotOwner',
+ 'GitRepositoryDeletionFault',
'GitRepositoryExists',
'GitRepositoryScanFault',
'GitTargetError',
@@ -347,6 +349,11 @@
GitRepositoryCreationException.__init__(self, message)
+@error_status(httplib.BAD_REQUEST)
+class CannotDeleteGitRepository(Exception):
+ """The Git repository cannot be deleted at this time."""
+
+
class GitRepositoryCreationForbidden(GitRepositoryCreationException):
"""A visibility policy forbids Git repository creation.
@@ -381,6 +388,10 @@
"""Raised when there is a fault scanning a repository."""
+class GitRepositoryDeletionFault(Exception):
+ """Raised when there is a fault deleting a repository."""
+
+
class GitTargetError(Exception):
"""Raised when there is an error determining a Git repository target."""
=== modified file 'lib/lp/code/githosting.py'
--- lib/lp/code/githosting.py 2015-04-30 17:10:28 +0000
+++ lib/lp/code/githosting.py 2015-05-19 11:33:34 +0000
@@ -15,6 +15,7 @@
from lp.code.errors import (
GitRepositoryCreationFault,
+ GitRepositoryDeletionFault,
GitRepositoryScanFault,
)
@@ -127,9 +128,23 @@
if response.status_code != 200:
raise GitRepositoryScanFault(
"Failed to get merge diff from Git repository: %s" %
- unicode(e))
+ response.text)
try:
return response.json()
except ValueError as e:
raise GitRepositoryScanFault(
"Failed to decode merge-diff response: %s" % unicode(e))
+
+ def delete(self, path, logger=None):
+ """Delete a repository."""
+ try:
+ if logger is not None:
+ logger.info("Deleting repository %s" % path)
+ response = self._makeSession().delete(
+ urljoin(self.endpoint, "/repo/%s" % path))
+ except Exception as e:
+ raise GitRepositoryDeletionFault(
+ "Failed to delete Git repository: %s" % unicode(e))
+ if response.status_code != 200:
+ raise GitRepositoryDeletionFault(
+ "Failed to delete Git repository: %s" % response.text)
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2015-04-30 21:49:59 +0000
+++ lib/lp/code/interfaces/branch.py 2015-05-19 11:33:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Branch interfaces."""
@@ -1167,15 +1167,9 @@
branch.
"""
+ @call_with(break_references=True)
@export_destructor_operation()
@operation_for_version('beta')
- def destroySelfBreakReferences():
- """Delete the specified branch.
-
- BranchRevisions associated with this branch will also be deleted as
- well as any items with mandatory references.
- """
-
def destroySelf(break_references=False):
"""Delete the specified branch.
=== modified file 'lib/lp/code/interfaces/gitjob.py'
--- lib/lp/code/interfaces/gitjob.py 2015-03-12 15:21:27 +0000
+++ lib/lp/code/interfaces/gitjob.py 2015-05-19 11:33:34 +0000
@@ -9,6 +9,8 @@
'IGitJob',
'IGitRefScanJob',
'IGitRefScanJobSource',
+ 'IReclaimGitRepositorySpaceJob',
+ 'IReclaimGitRepositorySpaceJobSource',
]
from lazr.restful.fields import Reference
@@ -16,6 +18,7 @@
Attribute,
Interface,
)
+from zope.schema import Text
from lp import _
from lp.code.interfaces.gitrepository import IGitRepository
@@ -35,7 +38,7 @@
repository = Reference(
title=_("The Git repository to use for this job."),
- schema=IGitRepository, required=True, readonly=True)
+ schema=IGitRepository, required=False, readonly=True)
metadata = Attribute(_("A dict of data about the job."))
@@ -51,3 +54,24 @@
:param repository: The database repository to scan.
"""
+
+
+class IReclaimGitRepositorySpaceJob(IRunnableJob):
+ """A Job that deletes a repository from storage after it has been
+ deleted from the database."""
+
+ repository_path = Text(
+ title=_("The storage path of the now-deleted repository."))
+
+
+class IReclaimGitRepositorySpaceJobSource(IJobSource):
+
+ def create(repository_name, repository_path):
+ """Construct a new object that implements
+ IReclaimGitRepositorySpaceJob.
+
+ :param repository_name: The unique name of the repository to remove
+ from storage.
+ :param repository_path: The storage path of the repository to remove
+ from storage.
+ """
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2015-05-19 11:33:34 +0000
@@ -449,6 +449,18 @@
and their subscriptions.
"""
+ # XXX cjwatson 2015-04-16: These names are too awful to set in stone by
+ # exporting them on the webservice; find better names before exporting.
+ landing_targets = Attribute(
+ "A collection of the merge proposals where this repository is the "
+ "source.")
+ landing_candidates = Attribute(
+ "A collection of the merge proposals where this repository is the "
+ "target.")
+ dependent_landings = Attribute(
+ "A collection of the merge proposals that are dependent on this "
+ "repository.")
+
def isRepositoryMergeable(other):
"""Is the other repository mergeable into this one (or vice versa)?"""
@@ -532,10 +544,35 @@
def setTarget(target, user):
"""Set the target of the repository."""
+ @export_read_operation()
+ @operation_for_version("devel")
+ def canBeDeleted():
+ """Can this repository be deleted in its current state?
+
+ A repository is considered deletable if it is not linked to any
+ merge proposals.
+ """
+
+ def getDeletionRequirements():
+ """Determine what is required to delete this branch.
+
+ :return: a dict of {object: (operation, reason)}, where object is the
+ object that must be deleted or altered, operation is either
+ "delete" or "alter", and reason is a string explaining why the
+ object needs to be touched.
+ """
+
+ @call_with(break_references=True)
@export_destructor_operation()
@operation_for_version("devel")
- def destroySelf():
- """Delete the specified repository."""
+ def destroySelf(break_references=False):
+ """Delete the specified repository.
+
+ :param break_references: If supplied, break any references to this
+ repository by deleting items with mandatory references and
+ NULLing other references.
+ :raise: CannotDeleteGitRepository if the repository cannot be deleted.
+ """
class IGitRepository(IGitRepositoryView, IGitRepositoryModerateAttributes,
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2015-04-30 21:49:59 +0000
+++ lib/lp/code/model/branch.py 2015-05-19 11:33:34 +0000
@@ -1250,10 +1250,6 @@
datetime.now(pytz.timezone('UTC'))
+ increment * 2 ** (self.mirror_failures - 1))
- def destroySelfBreakReferences(self):
- """See `IBranch`."""
- return self.destroySelf(break_references=True)
-
def _deleteBranchSubscriptions(self):
"""Delete subscriptions for this branch prior to deleting branch."""
subscriptions = Store.of(self).find(
=== modified file 'lib/lp/code/model/gitjob.py'
--- lib/lp/code/model/gitjob.py 2015-05-14 09:46:25 +0000
+++ lib/lp/code/model/gitjob.py 2015-05-19 11:33:34 +0000
@@ -7,6 +7,7 @@
'GitJob',
'GitJobType',
'GitRefScanJob',
+ 'ReclaimGitRepositorySpaceJob',
]
from lazr.delegates import delegates
@@ -19,6 +20,7 @@
Int,
JSON,
Reference,
+ SQL,
Store,
)
from zope.interface import (
@@ -32,6 +34,8 @@
IGitJob,
IGitRefScanJob,
IGitRefScanJobSource,
+ IReclaimGitRepositorySpaceJob,
+ IReclaimGitRepositorySpaceJobSource,
)
from lp.services.config import config
from lp.services.database.enumcol import EnumCol
@@ -63,6 +67,13 @@
This job scans a repository for its current list of references.
""")
+ RECLAIM_REPOSITORY_SPACE = DBItem(1, """
+ Reclaim repository space
+
+ This job removes a repository that has been deleted from the
+ database from storage.
+ """)
+
class GitJob(StormBase):
"""See `IGitJob`."""
@@ -74,7 +85,7 @@
job_id = Int(name='job', primary=True, allow_none=False)
job = Reference(job_id, 'Job.id')
- repository_id = Int(name='repository', allow_none=False)
+ repository_id = Int(name='repository', allow_none=True)
repository = Reference(repository_id, 'GitRepository.id')
job_type = EnumCol(enum=GitJobType, notNull=True)
@@ -97,7 +108,8 @@
self.repository = repository
self.job_type = job_type
self.metadata = metadata
- self.metadata["repository_name"] = repository.unique_name
+ if repository is not None:
+ self.metadata["repository_name"] = repository.unique_name
def makeDerived(self):
return GitJobDerived.makeSubclass(self)
@@ -205,3 +217,43 @@
log.info(
"Skipping repository %s because it has been deleted." %
self._cached_repository_name)
+
+
+class ReclaimGitRepositorySpaceJob(GitJobDerived):
+ """A Job that deletes a repository from storage after it has been
+ deleted from the database."""
+
+ implements(IReclaimGitRepositorySpaceJob)
+
+ classProvides(IReclaimGitRepositorySpaceJobSource)
+ class_job_type = GitJobType.RECLAIM_REPOSITORY_SPACE
+
+ config = config.IReclaimGitRepositorySpaceJobSource
+
+ @classmethod
+ def create(cls, repository_name, repository_path):
+ "See `IReclaimGitRepositorySpaceJobSource`."""
+ metadata = {
+ "repository_name": repository_name,
+ "repository_path": repository_path,
+ }
+ # The GitJob has a repository of None, as there is no repository
+ # left in the database to refer to.
+ start = SQL("CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + '7 days'")
+ git_job = GitJob(
+ None, cls.class_job_type, metadata, scheduled_start=start)
+ job = cls(git_job)
+ job.celeryRunOnCommit()
+ return job
+
+ def __init__(self, git_job):
+ super(ReclaimGitRepositorySpaceJob, self).__init__(git_job)
+ self._hosting_client = GitHostingClient(
+ config.codehosting.internal_git_api_endpoint)
+
+ @property
+ def repository_path(self):
+ return self.metadata["repository_path"]
+
+ def run(self):
+ self._hosting_client.delete(self.repository_path, logger=log)
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/gitrepository.py 2015-05-19 11:33:34 +0000
@@ -20,6 +20,7 @@
Coalesce,
Insert,
Join,
+ Not,
Or,
Select,
SQL,
@@ -39,8 +40,12 @@
from zope.component import getUtility
from zope.interface import implements
from zope.security.interfaces import Unauthorized
-from zope.security.proxy import removeSecurityProxy
+from zope.security.proxy import (
+ ProxyFactory,
+ removeSecurityProxy,
+ )
+from lp import _ as msg
from lp.app.enums import (
InformationType,
PRIVATE_INFORMATION_TYPES,
@@ -58,9 +63,13 @@
from lp.app.interfaces.services import IService
from lp.code.enums import GitObjectType
from lp.code.errors import (
+ CannotDeleteGitRepository,
GitDefaultConflict,
GitTargetError,
)
+from lp.code.interfaces.branchmergeproposal import (
+ BRANCH_MERGE_PROPOSAL_FINAL_STATES,
+ )
from lp.code.interfaces.gitcollection import (
IAllGitRepositories,
IGitCollection,
@@ -78,6 +87,7 @@
)
from lp.code.interfaces.revision import IRevisionSet
from lp.code.mail.branch import send_git_repository_modified_notifications
+from lp.code.model.branchmergeproposal import BranchMergeProposal
from lp.code.model.gitref import GitRef
from lp.code.model.gitsubscription import GitSubscription
from lp.registry.enums import PersonVisibility
@@ -756,6 +766,31 @@
recipients.add(subscription.person, subscription, rationale)
return recipients
+ @property
+ def landing_targets(self):
+ """See `IGitRepository`."""
+ return Store.of(self).find(
+ BranchMergeProposal,
+ BranchMergeProposal.source_git_repository == self)
+
+ @property
+ def landing_candidates(self):
+ """See `IGitRepository`."""
+ return Store.of(self).find(
+ BranchMergeProposal,
+ BranchMergeProposal.target_git_repository == self,
+ Not(BranchMergeProposal.queue_status.is_in(
+ BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+
+ @property
+ def dependent_landings(self):
+ """See `IGitRepository`."""
+ return Store.of(self).find(
+ BranchMergeProposal,
+ BranchMergeProposal.prerequisite_git_repository,
+ Not(BranchMergeProposal.queue_status.is_in(
+ BRANCH_MERGE_PROPOSAL_FINAL_STATES)))
+
def isRepositoryMergeable(self, other):
"""See `IGitRepository`."""
return self.namespace.areRepositoriesMergeable(other.namespace)
@@ -775,8 +810,164 @@
Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]))
return not jobs.is_empty()
- def destroySelf(self):
- raise NotImplementedError
+ def canBeDeleted(self):
+ """See `IGitRepository`."""
+ # Can't delete if the repository is associated with anything.
+ return len(self.getDeletionRequirements()) == 0
+
+ def _getDeletionRequirements(self):
+ """Determine what operations must be performed to delete this branch.
+
+ Two dictionaries are returned, one for items that must be deleted,
+ one for items that must be altered. The item in question is the
+ key, and the value is a user-facing string explaining why the item
+ is affected.
+
+ As well as the dictionaries, this method returns two list of callables
+ that may be called to perform the alterations and deletions needed.
+ """
+ alteration_operations = []
+ deletion_operations = []
+ # Merge proposals require their source and target repositories to
+ # exist.
+ for merge_proposal in self.landing_targets:
+ deletion_operations.append(
+ DeletionCallable(
+ merge_proposal,
+ msg("This repository is the source repository of this "
+ "merge proposal."),
+ merge_proposal.deleteProposal))
+ # Cannot use self.landing_candidates, because it ignores merged
+ # merge proposals.
+ for merge_proposal in BranchMergeProposal.selectBy(
+ target_git_repository=self):
+ deletion_operations.append(
+ DeletionCallable(
+ merge_proposal,
+ msg("This repository is the target repository of this "
+ "merge proposal."),
+ merge_proposal.deleteProposal))
+ for merge_proposal in BranchMergeProposal.selectBy(
+ prerequisite_git_repository=self):
+ alteration_operations.append(
+ ClearPrerequisiteRepository(merge_proposal))
+
+ return (alteration_operations, deletion_operations)
+
+ def getDeletionRequirements(self):
+ """See `IGitRepository`."""
+ alteration_operations, deletion_operations = (
+ self._getDeletionRequirements())
+ result = {
+ operation.affected_object: ("alter", operation.rationale)
+ for operation in alteration_operations}
+ # Deletion entries should overwrite alteration entries.
+ result.update({
+ operation.affected_object: ("delete", operation.rationale)
+ for operation in deletion_operations})
+ return result
+
+ def _breakReferences(self):
+ """Break all external references to this repository.
+
+ NULLable references will be NULLed. References which are not NULLable
+ will cause the item holding the reference to be deleted.
+
+ This function is guaranteed to perform the operations predicted by
+ getDeletionRequirements, because it uses the same backing function.
+ """
+ alteration_operations, deletion_operations = (
+ self._getDeletionRequirements())
+ for operation in alteration_operations:
+ operation()
+ for operation in deletion_operations:
+ operation()
+ Store.of(self).flush()
+
+ def _deleteRepositorySubscriptions(self):
+ """Delete subscriptions for this repository prior to deleting it."""
+ subscriptions = Store.of(self).find(
+ GitSubscription, GitSubscription.repository == self)
+ subscriptions.remove()
+
+ def _deleteJobs(self):
+ """Delete jobs for this repository prior to deleting it.
+
+ This deletion includes `GitJob`s associated with the branch.
+ """
+ # Circular import.
+ from lp.code.model.gitjob import GitJob
+
+ # Remove GitJobs.
+ affected_jobs = Select(
+ [GitJob.job_id],
+ And(GitJob.job == Job.id, GitJob.repository == self))
+ Store.of(self).find(Job, Job.id.is_in(affected_jobs)).remove()
+
+ def destroySelf(self, break_references=False):
+ """See `IGitRepository`."""
+ # Circular import.
+ from lp.code.interfaces.gitjob import (
+ IReclaimGitRepositorySpaceJobSource,
+ )
+
+ if break_references:
+ self._breakReferences()
+ if not self.canBeDeleted():
+ raise CannotDeleteGitRepository(
+ "Cannot delete Git repository: %s" % self.unique_name)
+
+ self.refs.remove()
+ self._deleteRepositorySubscriptions()
+ self._deleteJobs()
+
+ # Now destroy the repository.
+ repository_name = self.unique_name
+ repository_path = self.getInternalPath()
+ Store.of(self).remove(self)
+ # And now create a job to remove the repository from storage when
+ # it's done.
+ getUtility(IReclaimGitRepositorySpaceJobSource).create(
+ repository_name, repository_path)
+
+
+class DeletionOperation:
+ """Represent an operation to perform as part of branch deletion."""
+
+ def __init__(self, affected_object, rationale):
+ self.affected_object = ProxyFactory(affected_object)
+ self.rationale = rationale
+
+ def __call__(self):
+ """Perform the deletion operation."""
+ raise NotImplementedError(DeletionOperation.__call__)
+
+
+class DeletionCallable(DeletionOperation):
+ """Deletion operation that invokes a callable."""
+
+ def __init__(self, affected_object, rationale, func):
+ super(DeletionCallable, self).__init__(affected_object, rationale)
+ self.func = func
+
+ def __call__(self):
+ self.func()
+
+
+class ClearPrerequisiteRepository(DeletionOperation):
+ """Delete operation that clears a merge proposal's prerequisite
+ repository."""
+
+ def __init__(self, merge_proposal):
+ DeletionOperation.__init__(
+ self, merge_proposal,
+ msg("This repository is the prerequisite repository of this merge "
+ "proposal."))
+
+ def __call__(self):
+ self.affected_object.prerequisite_git_repository = None
+ self.affected_object.prerequisite_git_path = None
+ self.affected_object.prerequisite_git_commit_sha1 = None
class GitRepositorySet:
=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
--- lib/lp/code/model/tests/test_gitjob.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/tests/test_gitjob.py 2015-05-19 11:33:34 +0000
@@ -16,18 +16,22 @@
MatchesSetwise,
MatchesStructure,
)
+from zope.security.proxy import removeSecurityProxy
from lp.code.enums import GitObjectType
from lp.code.interfaces.gitjob import (
IGitJob,
IGitRefScanJob,
+ IReclaimGitRepositorySpaceJob,
)
from lp.code.model.gitjob import (
GitJob,
GitJobDerived,
GitJobType,
GitRefScanJob,
+ ReclaimGitRepositorySpaceJob,
)
+from lp.services.database.constants import UTC_NOW
from lp.testing import (
TestCaseWithFactory,
time_counter,
@@ -65,7 +69,10 @@
self.assertIsNone(derived.getOopsMailController("x"))
-class TestGitRefScanJobMixin:
+class TestGitRefScanJob(TestCaseWithFactory):
+ """Tests for `GitRefScanJob`."""
+
+ layer = LaunchpadZopelessLayer
@staticmethod
def makeFakeRefs(paths):
@@ -107,12 +114,6 @@
for path in paths]
self.assertThat(refs, MatchesSetwise(*matchers))
-
-class TestGitRefScanJob(TestGitRefScanJobMixin, TestCaseWithFactory):
- """Tests for `GitRefScanJob`."""
-
- layer = LaunchpadZopelessLayer
-
def test_provides_interface(self):
# `GitRefScanJob` objects provide `IGitRefScanJob`.
repository = self.factory.makeGitRepository()
@@ -156,5 +157,64 @@
self.assertEqual([], list(repository.refs))
-# XXX cjwatson 2015-03-12: We should test that the job works via Celery too,
+class TestReclaimGitRepositorySpaceJob(TestCaseWithFactory):
+ """Tests for `ReclaimGitRepositorySpaceJob`."""
+
+ layer = LaunchpadZopelessLayer
+
+ def test_provides_interface(self):
+ # `ReclaimGitRepositorySpaceJob` objects provide
+ # `IReclaimGitRepositorySpaceJob`.
+ self.assertProvides(
+ ReclaimGitRepositorySpaceJob.create("/~owner/+git/gone", "1"),
+ IReclaimGitRepositorySpaceJob)
+
+ def test___repr__(self):
+ # `ReclaimGitRepositorySpaceJob` objects have an informative
+ # __repr__.
+ name = "/~owner/+git/gone"
+ job = ReclaimGitRepositorySpaceJob.create(name, "1")
+ self.assertEqual(
+ "<ReclaimGitRepositorySpaceJob for %s>" % name, repr(job))
+
+ def test_scheduled_in_future(self):
+ # A freshly created ReclaimGitRepositorySpaceJob is scheduled to run
+ # in a week's time.
+ job = ReclaimGitRepositorySpaceJob.create("/~owner/+git/gone", "1")
+ self.assertEqual(
+ timedelta(days=7), job.job.scheduled_start - job.job.date_created)
+
+ def test_stores_name_and_path(self):
+ # An instance of ReclaimGitRepositorySpaceJob stores the name and
+ # path of the repository that has been deleted.
+ name = "/~owner/+git/gone"
+ path = "1"
+ job = ReclaimGitRepositorySpaceJob.create(name, path)
+ self.assertEqual(name, job._cached_repository_name)
+ self.assertEqual(path, job.repository_path)
+
+ def makeJobReady(self, job):
+ """Force `job` to be scheduled to run now.
+
+ New `ReclaimGitRepositorySpaceJob`s are scheduled to run a week
+ after creation, so to be able to test running the job we have to
+ force them to be scheduled now.
+ """
+ removeSecurityProxy(job).job.scheduled_start = UTC_NOW
+
+ def test_run(self):
+ # Running a job to reclaim space sends a request to the hosting
+ # service.
+ name = "/~owner/+git/gone"
+ path = "1"
+ job = ReclaimGitRepositorySpaceJob.create(name, path)
+ self.makeJobReady(job)
+ [job] = list(ReclaimGitRepositorySpaceJob.iterReady())
+ with dbuser("reclaim-branch-space"):
+ job._hosting_client.delete = FakeMethod()
+ job.run()
+ self.assertEqual([(path,)], job._hosting_client.delete.extract_args())
+
+
+# XXX cjwatson 2015-03-12: We should test that the jobs work via Celery too,
# but that isn't feasible until we have a proper turnip fixture.
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2015-05-19 11:33:34 +0000
@@ -14,6 +14,8 @@
from bzrlib import urlutils
from lazr.lifecycle.event import ObjectModifiedEvent
from lazr.lifecycle.snapshot import Snapshot
+from sqlobject import SQLObjectNotFound
+from storm.store import Store
import transaction
import pytz
from testtools.matchers import (
@@ -27,6 +29,7 @@
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
+from lp import _
from lp.app.enums import (
InformationType,
PRIVATE_INFORMATION_TYPES,
@@ -40,6 +43,7 @@
GitObjectType,
)
from lp.code.errors import (
+ CannotDeleteGitRepository,
GitRepositoryCreatorNotMemberOfOwnerTeam,
GitRepositoryCreatorNotOwner,
GitRepositoryExists,
@@ -47,6 +51,7 @@
)
from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository
from lp.code.interfaces.gitjob import IGitRefScanJobSource
+from lp.code.interfaces.gitlookup import IGitLookup
from lp.code.interfaces.gitnamespace import (
IGitNamespacePolicy,
IGitNamespaceSet,
@@ -56,7 +61,19 @@
IGitRepositorySet,
)
from lp.code.interfaces.revision import IRevisionSet
-from lp.code.model.gitrepository import GitRepository
+from lp.code.model.branchmergeproposal import BranchMergeProposal
+from lp.code.model.codereviewcomment import CodeReviewComment
+from lp.code.model.gitjob import (
+ GitJob,
+ GitJobType,
+ ReclaimGitRepositorySpaceJob,
+ )
+from lp.code.model.gitrepository import (
+ ClearPrerequisiteRepository,
+ DeletionCallable,
+ DeletionOperation,
+ GitRepository,
+ )
from lp.code.xmlrpc.git import GitAPI
from lp.registry.enums import (
BranchSharingPolicy,
@@ -84,6 +101,7 @@
ANONYMOUS,
api_url,
celebrity_logged_in,
+ login_person,
person_logged_in,
TestCaseWithFactory,
verifyObject,
@@ -93,6 +111,7 @@
from lp.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
+ LaunchpadZopelessLayer,
)
from lp.testing.pages import webservice_for_person
@@ -284,6 +303,305 @@
repository.getRepositoryIdentities())
+class TestGitRepositoryDeletion(TestCaseWithFactory):
+ """Test the different cases that make a repository deletable or not."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestGitRepositoryDeletion, self).setUp()
+ self.user = self.factory.makePerson()
+ self.project = self.factory.makeProduct(owner=self.user)
+ self.repository = self.factory.makeGitRepository(
+ name=u"to-delete", owner=self.user, target=self.project)
+ [self.ref] = self.factory.makeGitRefs(repository=self.repository)
+ # The owner of the repository is subscribed to the repository when
+ # it is created. The tests here assume no initial connections, so
+ # unsubscribe the repository owner here.
+ self.repository.unsubscribe(
+ self.repository.owner, self.repository.owner)
+ # Make sure that the tests all flush the database changes.
+ self.addCleanup(Store.of(self.repository).flush)
+ login_person(self.user)
+
+ def test_deletable(self):
+ # A newly created repository can be deleted without any problems.
+ self.assertTrue(
+ self.repository.canBeDeleted(),
+ "A newly created repository should be able to be deleted.")
+ repository_id = self.repository.id
+ self.repository.destroySelf()
+ self.assertIsNone(
+ getUtility(IGitLookup).get(repository_id),
+ "The repository has not been deleted.")
+
+ def test_subscription_does_not_disable_deletion(self):
+ # A repository that has a subscription can be deleted.
+ self.repository.subscribe(
+ self.user, BranchSubscriptionNotificationLevel.NOEMAIL, None,
+ CodeReviewNotificationLevel.NOEMAIL, self.user)
+ self.assertTrue(self.repository.canBeDeleted())
+
+ def test_landing_target_disables_deletion(self):
+ # A repository with a landing target cannot be deleted.
+ [merge_target] = self.factory.makeGitRefs(
+ name=u"landing-target", owner=self.user, target=self.project)
+ self.ref.addLandingTarget(self.user, merge_target)
+ self.assertFalse(
+ self.repository.canBeDeleted(),
+ "A repository with a landing target is not deletable.")
+ self.assertRaises(
+ CannotDeleteGitRepository, self.repository.destroySelf)
+
+ def test_landing_candidate_disables_deletion(self):
+ # A repository with a landing candidate cannot be deleted.
+ [merge_source] = self.factory.makeGitRefs(
+ name=u"landing-candidate", owner=self.user, target=self.project)
+ merge_source.addLandingTarget(self.user, self.ref)
+ self.assertFalse(
+ self.repository.canBeDeleted(),
+ "A repository with a landing candidate is not deletable.")
+ self.assertRaises(
+ CannotDeleteGitRepository, self.repository.destroySelf)
+
+ def test_prerequisite_repository_disables_deletion(self):
+ # A repository that is a prerequisite repository cannot be deleted.
+ [merge_source] = self.factory.makeGitRefs(
+ name=u"landing-candidate", owner=self.user, target=self.project)
+ [merge_target] = self.factory.makeGitRefs(
+ name=u"landing-target", owner=self.user, target=self.project)
+ merge_source.addLandingTarget(self.user, merge_target, self.ref)
+ self.assertFalse(
+ self.repository.canBeDeleted(),
+ "A repository with a prerequisite target is not deletable.")
+ self.assertRaises(
+ CannotDeleteGitRepository, self.repository.destroySelf)
+
+ def test_related_GitJobs_deleted(self):
+ # A repository with an associated job will delete those jobs.
+ repository = self.factory.makeGitRepository()
+ GitAPI(None, None).notify(repository.getInternalPath())
+ store = Store.of(repository)
+ repository.destroySelf()
+ # Need to commit the transaction to fire off the constraint checks.
+ transaction.commit()
+ jobs = store.find(GitJob, GitJob.job_type == GitJobType.REF_SCAN)
+ self.assertEqual([], list(jobs))
+
+ def test_creates_job_to_reclaim_space(self):
+ # When a repository is deleted from the database, a job is created
+ # to remove the repository from disk as well.
+ repository = self.factory.makeGitRepository()
+ repository_path = repository.getInternalPath()
+ store = Store.of(repository)
+ repository.destroySelf()
+ jobs = store.find(
+ GitJob,
+ GitJob.job_type == GitJobType.RECLAIM_REPOSITORY_SPACE)
+ self.assertEqual(
+ [repository_path],
+ [ReclaimGitRepositorySpaceJob(job).repository_path
+ for job in jobs])
+
+ def test_destroySelf_with_inline_comments_draft(self):
+ # Draft inline comments related to a deleted repository (source or
+ # target MP repository) also get removed.
+ merge_proposal = self.factory.makeBranchMergeProposalForGit(
+ registrant=self.user, target_ref=self.ref)
+ preview_diff = self.factory.makePreviewDiff(
+ merge_proposal=merge_proposal)
+ transaction.commit()
+ merge_proposal.saveDraftInlineComment(
+ previewdiff_id=preview_diff.id,
+ person=self.user,
+ comments={"1": "Should vanish."})
+ self.repository.destroySelf(break_references=True)
+
+ def test_destroySelf_with_inline_comments_published(self):
+ # Published inline comments related to a deleted repository (source
+ # or target MP repository) also get removed.
+ merge_proposal = self.factory.makeBranchMergeProposalForGit(
+ registrant=self.user, target_ref=self.ref)
+ preview_diff = self.factory.makePreviewDiff(
+ merge_proposal=merge_proposal)
+ transaction.commit()
+ merge_proposal.createComment(
+ owner=self.user,
+ subject="Delete me!",
+ previewdiff_id=preview_diff.id,
+ inline_comments={"1": "Must disappear."},
+ )
+ self.repository.destroySelf(break_references=True)
+
+
+class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
+ """Test determination and application of repository deletion
+ consequences."""
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestGitRepositoryDeletionConsequences, self).setUp(
+ user="test@xxxxxxxxxxxxx")
+ self.repository = self.factory.makeGitRepository()
+ [self.ref] = self.factory.makeGitRefs(repository=self.repository)
+ # The owner of the repository is subscribed to the repository when
+ # it is created. The tests here assume no initial connections, so
+ # unsubscribe the repository owner here.
+ self.repository.unsubscribe(
+ self.repository.owner, self.repository.owner)
+
+ def test_plain_repository(self):
+ # A fresh repository has no deletion requirements.
+ self.assertEqual({}, self.repository.getDeletionRequirements())
+
+ def makeMergeProposals(self):
+ # Produce a merge proposal for testing purposes.
+ [merge_target] = self.factory.makeGitRefs(target=self.ref.target)
+ [merge_prerequisite] = self.factory.makeGitRefs(target=self.ref.target)
+ # Remove the implicit subscriptions.
+ merge_target.repository.unsubscribe(
+ merge_target.owner, merge_target.owner)
+ merge_prerequisite.repository.unsubscribe(
+ merge_prerequisite.owner, merge_prerequisite.owner)
+ merge_proposal1 = self.ref.addLandingTarget(
+ self.ref.owner, merge_target, merge_prerequisite)
+ # Disable this merge proposal, to allow creating a new identical one.
+ lp_admins = getUtility(ILaunchpadCelebrities).admin
+ merge_proposal1.rejectBranch(lp_admins, "null:")
+ merge_proposal2 = self.ref.addLandingTarget(
+ self.ref.owner, merge_target, merge_prerequisite)
+ return merge_proposal1, merge_proposal2
+
+ def test_repository_with_merge_proposal(self):
+ # Ensure that deletion requirements with a merge proposal are right.
+ #
+ # Each repository related to the merge proposal is tested to ensure
+ # it produces a unique, correct result.
+ merge_proposal1, merge_proposal2 = self.makeMergeProposals()
+ self.assertEqual(
+ {
+ merge_proposal1:
+ ("delete",
+ _("This repository is the source repository of this merge "
+ "proposal.")),
+ merge_proposal2:
+ ("delete",
+ _("This repository is the source repository of this merge "
+ "proposal.")),
+ },
+ self.repository.getDeletionRequirements())
+ target = merge_proposal1.target_git_repository
+ self.assertEqual(
+ {
+ merge_proposal1:
+ ("delete",
+ _("This repository is the target repository of this merge "
+ "proposal.")),
+ merge_proposal2:
+ ("delete",
+ _("This repository is the target repository of this merge "
+ "proposal.")),
+ },
+ target.getDeletionRequirements())
+ prerequisite = merge_proposal1.prerequisite_git_repository
+ self.assertEqual(
+ {
+ merge_proposal1:
+ ("alter",
+ _("This repository is the prerequisite repository of this "
+ "merge proposal.")),
+ merge_proposal2:
+ ("alter",
+ _("This repository is the prerequisite repository of this "
+ "merge proposal.")),
+ },
+ prerequisite.getDeletionRequirements())
+
+ def test_delete_merge_proposal_source(self):
+ # Merge proposal source repositories can be deleted with
+ # break_references.
+ merge_proposal1, merge_proposal2 = self.makeMergeProposals()
+ merge_proposal1_id = merge_proposal1.id
+ BranchMergeProposal.get(merge_proposal1_id)
+ self.repository.destroySelf(break_references=True)
+ self.assertRaises(
+ SQLObjectNotFound, BranchMergeProposal.get, merge_proposal1_id)
+
+ def test_delete_merge_proposal_target(self):
+ # Merge proposal target repositories can be deleted with
+ # break_references.
+ merge_proposal1, merge_proposal2 = self.makeMergeProposals()
+ merge_proposal1_id = merge_proposal1.id
+ BranchMergeProposal.get(merge_proposal1_id)
+ merge_proposal1.target_git_repository.destroySelf(
+ break_references=True)
+ self.assertRaises(SQLObjectNotFound,
+ BranchMergeProposal.get, merge_proposal1_id)
+
+ def test_delete_merge_proposal_prerequisite(self):
+ # Merge proposal prerequisite repositories can be deleted with
+ # break_references.
+ merge_proposal1, merge_proposal2 = self.makeMergeProposals()
+ merge_proposal1.prerequisite_git_repository.destroySelf(
+ break_references=True)
+ self.assertIsNone(merge_proposal1.prerequisite_git_repository)
+
+ def test_delete_source_CodeReviewComment(self):
+ # Deletion of source repositories that have CodeReviewComments works.
+ comment = self.factory.makeCodeReviewComment(git=True)
+ comment_id = comment.id
+ repository = comment.branch_merge_proposal.source_git_repository
+ repository.destroySelf(break_references=True)
+ self.assertRaises(
+ SQLObjectNotFound, CodeReviewComment.get, comment_id)
+
+ def test_delete_target_CodeReviewComment(self):
+ # Deletion of target repositories that have CodeReviewComments works.
+ comment = self.factory.makeCodeReviewComment(git=True)
+ comment_id = comment.id
+ repository = comment.branch_merge_proposal.target_git_repository
+ repository.destroySelf(break_references=True)
+ self.assertRaises(
+ SQLObjectNotFound, CodeReviewComment.get, comment_id)
+
+ def test_sourceBranchWithCodeReviewVoteReference(self):
+ # break_references handles CodeReviewVoteReference source repository.
+ merge_proposal = self.factory.makeBranchMergeProposalForGit()
+ merge_proposal.nominateReviewer(
+ self.factory.makePerson(), self.factory.makePerson())
+ merge_proposal.source_git_repository.destroySelf(break_references=True)
+
+ def test_targetBranchWithCodeReviewVoteReference(self):
+ # break_references handles CodeReviewVoteReference target repository.
+ merge_proposal = self.factory.makeBranchMergeProposalForGit()
+ merge_proposal.nominateReviewer(
+ self.factory.makePerson(), self.factory.makePerson())
+ merge_proposal.target_git_repository.destroySelf(break_references=True)
+
+ def test_ClearPrerequisiteRepository(self):
+ # ClearPrerequisiteRepository.__call__ must clear the prerequisite
+ # repository.
+ merge_proposal = removeSecurityProxy(self.makeMergeProposals()[0])
+ with person_logged_in(
+ merge_proposal.prerequisite_git_repository.owner):
+ ClearPrerequisiteRepository(merge_proposal)()
+ self.assertIsNone(merge_proposal.prerequisite_git_repository)
+
+ def test_DeletionOperation(self):
+ # DeletionOperation.__call__ is not implemented.
+ self.assertRaises(NotImplementedError, DeletionOperation("a", "b"))
+
+ def test_DeletionCallable(self):
+ # DeletionCallable must invoke the callable.
+ merge_proposal = self.factory.makeBranchMergeProposalForGit()
+ merge_proposal_id = merge_proposal.id
+ DeletionCallable(
+ merge_proposal, "blah", merge_proposal.deleteProposal)()
+ self.assertRaises(
+ SQLObjectNotFound, BranchMergeProposal.get, merge_proposal_id)
+
+
class TestGitRepositoryModifications(TestCaseWithFactory):
"""Tests for Git repository modification notifications."""
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2015-04-22 16:00:09 +0000
+++ lib/lp/services/config/schema-lazr.conf 2015-05-19 11:33:34 +0000
@@ -1818,6 +1818,10 @@
module: lp.code.interfaces.branchjob
dbuser: reclaim-branch-space
+[IReclaimGitRepositorySpaceJobSource]
+module: lp.code.interfaces.gitjob
+dbuser: reclaim-branch-space
+
[IRemoveArtifactSubscriptionsJobSource]
module: lp.registry.interfaces.sharingjob
dbuser: sharing-jobs
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2015-05-19 00:41:13 +0000
+++ lib/lp/testing/factory.py 2015-05-19 11:33:34 +0000
@@ -1567,10 +1567,13 @@
Diff.fromFile(StringIO(diff_text), len(diff_text)))
def makePreviewDiff(self, conflicts=u'', merge_proposal=None,
- date_created=None, size='small'):
+ date_created=None, size='small', git=False):
diff = self.makeDiff(size)
if merge_proposal is None:
- merge_proposal = self.makeBranchMergeProposal()
+ if git:
+ merge_proposal = self.makeBranchMergeProposalForGit()
+ else:
+ merge_proposal = self.makeBranchMergeProposal()
preview_diff = PreviewDiff()
preview_diff.branch_merge_proposal = merge_proposal
preview_diff.conflicts = conflicts
@@ -2438,7 +2441,8 @@
def makeCodeReviewComment(self, sender=None, subject=None, body=None,
vote=None, vote_tag=None, parent=None,
- merge_proposal=None, date_created=DEFAULT):
+ merge_proposal=None, date_created=DEFAULT,
+ git=False):
if sender is None:
sender = self.makePerson()
if subject is None:
@@ -2448,6 +2452,9 @@
if merge_proposal is None:
if parent:
merge_proposal = parent.branch_merge_proposal
+ elif git:
+ merge_proposal = self.makeBranchMergeProposalForGit(
+ registrant=sender)
else:
merge_proposal = self.makeBranchMergeProposal(
registrant=sender)
@@ -2456,8 +2463,11 @@
sender, subject, body, vote, vote_tag, parent,
_date_created=date_created)
- def makeCodeReviewVoteReference(self):
- bmp = removeSecurityProxy(self.makeBranchMergeProposal())
+ def makeCodeReviewVoteReference(self, git=False):
+ if git:
+ bmp = removeSecurityProxy(self.makeBranchMergeProposalForGit())
+ else:
+ bmp = removeSecurityProxy(self.makeBranchMergeProposal())
candidate = self.makePerson()
return bmp.nominateReviewer(candidate, bmp.registrant)
Follow ups
