launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19162
[Merge] lp:~cjwatson/launchpad/snap-source-deletion into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-source-deletion into lp:launchpad.
Commit message:
Add Branch.snaps and GitRepository.snaps. Delete snap branches when their source branch/repository is deleted.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1476405 in Launchpad itself: "Add support for building snaps"
https://bugs.launchpad.net/launchpad/+bug/1476405
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-source-deletion/+merge/266866
Add Branch.snaps and GitRepository.snaps. Delete snap branches when their source branch/repository is deleted.
This will make it feasible to add the tests requested in a review comment on https://code.launchpad.net/~cjwatson/launchpad/snap-build-behaviour/+merge/266736.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-source-deletion into lp:launchpad.
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2015-05-26 12:18:12 +0000
+++ lib/lp/code/interfaces/branch.py 2015-08-04 13:10:52 +0000
@@ -105,6 +105,7 @@
structured,
)
from lp.services.webapp.interfaces import ITableBatchNavigator
+from lp.snappy.interfaces.hassnaps import IHasSnaps
DEFAULT_BRANCH_STATUS_IN_LISTING = (
@@ -275,7 +276,7 @@
class IBranchView(IHasOwner, IHasBranchTarget, IHasMergeProposals,
- IHasRecipes):
+ IHasRecipes, IHasSnaps):
"""IBranch attributes that require launchpad.View permission."""
id = Int(title=_('ID'), readonly=True, required=True)
=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py 2015-07-01 07:58:25 +0000
+++ lib/lp/code/interfaces/gitrepository.py 2015-08-04 13:10:52 +0000
@@ -78,6 +78,7 @@
PublicPersonChoice,
)
from lp.services.webhooks.interfaces import IWebhookTarget
+from lp.snappy.interfaces.hassnaps import IHasSnaps
GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE = _(
@@ -117,7 +118,7 @@
return True
-class IGitRepositoryView(Interface):
+class IGitRepositoryView(IHasSnaps):
"""IGitRepository attributes that require launchpad.View permission."""
id = Int(title=_("ID"), readonly=True, required=True)
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2015-07-08 16:05:11 +0000
+++ lib/lp/code/model/branch.py 2015-08-04 13:10:52 +0000
@@ -179,6 +179,7 @@
from lp.services.propertycache import cachedproperty
from lp.services.webapp import urlappend
from lp.services.webapp.authorization import check_permission
+from lp.snappy.interfaces.snap import ISnapSet
@implementer(IBranch, IPrivacy, IInformationType)
@@ -814,6 +815,8 @@
deletion_operations.extend(
DeletionCallable.forSourcePackageRecipe(recipe)
for recipe in self.recipes)
+ alteration_operations.extend(
+ ClearSnapBranch(snap, self) for snap in self.snaps)
return (alteration_operations, deletion_operations)
def deletionRequirements(self):
@@ -1420,6 +1423,11 @@
hook = SourcePackageRecipe.preLoadDataForSourcePackageRecipes
return DecoratedResultSet(self._recipes, pre_iter_hook=hook)
+ @property
+ def snaps(self):
+ """See `IHasSnaps`."""
+ return getUtility(ISnapSet).findByBranch(self)
+
class DeletionOperation:
"""Represent an operation to perform as part of branch deletion."""
@@ -1515,6 +1523,19 @@
CodeImportSet().delete(self.affected_object)
+class ClearSnapBranch(DeletionOperation):
+ """Deletion operation that clears a snap package's branch."""
+
+ def __init__(self, snap, branch):
+ DeletionOperation.__init__(
+ self, snap, _('This snap package uses this branch.'))
+ self.branch = branch
+
+ def __call__(self):
+ if self.affected_object.branch == self.branch:
+ self.affected_object.branch = None
+
+
@implementer(IBranchSet)
class BranchSet:
"""The set of all branches."""
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-07-17 07:34:21 +0000
+++ lib/lp/code/model/gitrepository.py 2015-08-04 13:10:52 +0000
@@ -148,6 +148,7 @@
from lp.services.webapp.authorization import available_with_permission
from lp.services.webhooks.interfaces import IWebhookSource
from lp.services.webhooks.model import WebhookTargetMixin
+from lp.snappy.interfaces.snap import ISnapSet
object_type_map = {
@@ -961,6 +962,11 @@
self._markProposalMerged(
proposal, merged_revision_id, logger=logger)
+ @property
+ def snaps(self):
+ """See `IHasSnaps`."""
+ return getUtility(ISnapSet).findByGitRepository(self)
+
def canBeDeleted(self):
"""See `IGitRepository`."""
# Can't delete if the repository is associated with anything.
@@ -1002,6 +1008,8 @@
prerequisite_git_repository=self):
alteration_operations.append(
ClearPrerequisiteRepository(merge_proposal))
+ alteration_operations.extend(
+ ClearSnapRepository(snap, self) for snap in self.snaps)
return (alteration_operations, deletion_operations)
@@ -1127,6 +1135,20 @@
self.affected_object.prerequisite_git_commit_sha1 = None
+class ClearSnapRepository(DeletionOperation):
+ """Deletion operation that clears a snap package's repository."""
+
+ def __init__(self, snap, repository):
+ DeletionOperation.__init__(
+ self, snap, msg("This snap package uses this repository."))
+ self.repository = repository
+
+ def __call__(self):
+ if self.affected_object.git_repository == self.repository:
+ self.affected_object.git_repository = None
+ self.affected_object.git_path = None
+
+
@implementer(IGitRepositorySet)
class GitRepositorySet:
"""See `IGitRepositorySet`."""
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2015-07-24 09:16:50 +0000
+++ lib/lp/code/model/tests/test_branch.py 2015-08-04 13:10:52 +0000
@@ -72,7 +72,6 @@
from lp.code.interfaces.branchjob import (
IBranchScanJobSource,
IBranchUpgradeJobSource,
- IReclaimBranchSpaceJobSource,
)
from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.interfaces.branchmergeproposal import (
@@ -93,6 +92,7 @@
ClearDependentBranch,
ClearOfficialPackageBranch,
ClearSeriesBranch,
+ ClearSnapBranch,
DeleteCodeImport,
DeletionCallable,
DeletionOperation,
@@ -143,6 +143,7 @@
IOpenLaunchBag,
OAuthPermission,
)
+from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
from lp.testing import (
admin_logged_in,
ANONYMOUS,
@@ -1672,6 +1673,24 @@
self.factory.makePerson())
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
+ # indicate this.
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ snap = self.factory.makeSnap(branch=self.branch)
+ self.assertEqual(
+ {snap: ('alter', _('This snap package uses this branch.'))},
+ self.branch.deletionRequirements())
+
+ def test_snap_deletion(self):
+ # break_references allows deleting a branch used by a snap package.
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ snap1 = self.factory.makeSnap(branch=self.branch)
+ snap2 = self.factory.makeSnap(branch=self.branch)
+ self.branch.destroySelf(break_references=True)
+ self.assertIsNone(snap1.branch)
+ self.assertIsNone(snap2.branch)
+
def test_ClearDependentBranch(self):
"""ClearDependent.__call__ must clear the prerequisite branch."""
merge_proposal = removeSecurityProxy(self.makeMergeProposals()[0])
@@ -1701,6 +1720,13 @@
ClearSeriesBranch(series, self.branch)()
self.assertEqual(None, series.branch)
+ def test_ClearSnapBranch(self):
+ """ClearSnapBranch.__call__ must clear the branch."""
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ snap = self.factory.makeSnap(branch=self.branch)
+ ClearSnapBranch(snap, self.branch)()
+ self.assertIsNone(snap.branch)
+
def test_DeletionOperation(self):
"""DeletionOperation.__call__ is not implemented."""
self.assertRaises(NotImplementedError, DeletionOperation('a', 'b'))
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2015-07-17 07:34:21 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py 2015-08-04 13:10:52 +0000
@@ -81,6 +81,7 @@
)
from lp.code.model.gitrepository import (
ClearPrerequisiteRepository,
+ ClearSnapRepository,
DeletionCallable,
DeletionOperation,
GitRepository,
@@ -104,12 +105,14 @@
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.database.interfaces import IStore
+from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
from lp.services.job.runner import JobRunner
from lp.services.mail import stub
from lp.services.webapp.authorization import check_permission
from lp.services.webapp.interfaces import OAuthPermission
+from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
from lp.testing import (
admin_logged_in,
ANONYMOUS,
@@ -620,6 +623,30 @@
self.factory.makePerson(), self.factory.makePerson())
merge_proposal.target_git_repository.destroySelf(break_references=True)
+ def test_snap_requirements(self):
+ # If a repository is used by a snap package, the deletion
+ # requirements indicate this.
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ [ref] = self.factory.makeGitRefs()
+ snap = self.factory.makeSnap(git_ref=ref)
+ self.assertEqual(
+ {snap: ("alter", _("This snap package uses this repository."))},
+ ref.repository.getDeletionRequirements())
+
+ def test_snap_deletion(self):
+ # break_references allows deleting a repository used by a snap package.
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ repository = self.factory.makeGitRepository()
+ [ref1, ref2] = self.factory.makeGitRefs(
+ repository=repository, paths=[u"refs/heads/1", u"refs/heads/2"])
+ snap1 = self.factory.makeSnap(git_ref=ref1)
+ snap2 = self.factory.makeSnap(git_ref=ref2)
+ repository.destroySelf(break_references=True)
+ self.assertIsNone(snap1.git_repository)
+ self.assertIsNone(snap1.git_path)
+ self.assertIsNone(snap2.git_repository)
+ self.assertIsNone(snap2.git_path)
+
def test_ClearPrerequisiteRepository(self):
# ClearPrerequisiteRepository.__call__ must clear the prerequisite
# repository.
@@ -629,6 +656,15 @@
ClearPrerequisiteRepository(merge_proposal)()
self.assertIsNone(merge_proposal.prerequisite_git_repository)
+ def test_ClearSnapRepository(self):
+ # ClearSnapRepository.__call__ must clear the repository and path.
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+ [ref] = self.factory.makeGitRefs()
+ snap = self.factory.makeSnap(git_ref=ref)
+ ClearSnapRepository(snap, ref.repository)()
+ self.assertIsNone(snap.git_repository)
+ self.assertIsNone(snap.git_path)
+
def test_DeletionOperation(self):
# DeletionOperation.__call__ is not implemented.
self.assertRaises(NotImplementedError, DeletionOperation("a", "b"))
=== added file 'lib/lp/snappy/interfaces/hassnaps.py'
--- lib/lp/snappy/interfaces/hassnaps.py 1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/interfaces/hassnaps.py 2015-08-04 13:10:52 +0000
@@ -0,0 +1,31 @@
+# Copyright 2015 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface definitions for IHasSnaps."""
+
+__metaclass__ = type
+__all__ = [
+ 'IHasSnaps',
+ ]
+
+
+from lazr.lifecycle.snapshot import doNotSnapshot
+from lazr.restful.declarations import exported
+from lazr.restful.fields import (
+ CollectionField,
+ Reference,
+ )
+from zope.interface import Interface
+
+from lp import _
+
+
+class IHasSnaps(Interface):
+ """An object that has snap packages."""
+
+ snaps = exported(doNotSnapshot(
+ CollectionField(
+ title=_("All snap packages associated with the object."),
+ # Really ISnap.
+ value_type=Reference(schema=Interface),
+ readonly=True)))
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2015-08-03 13:16:48 +0000
+++ lib/lp/snappy/interfaces/snap.py 2015-08-04 13:10:52 +0000
@@ -347,6 +347,12 @@
def findByPerson(owner):
"""Return all snap packages with the given `owner`."""
+ def findByBranch(branch):
+ """Return all snap packages for the given Bazaar branch."""
+
+ def findByGitRepository(repository):
+ """Return all snap packages for the given Git repository."""
+
@collection_default_content()
def empty_list():
"""Return an empty collection of snap packages.
=== modified file 'lib/lp/snappy/interfaces/webservice.py'
--- lib/lp/snappy/interfaces/webservice.py 2015-08-03 11:45:43 +0000
+++ lib/lp/snappy/interfaces/webservice.py 2015-08-04 13:10:52 +0000
@@ -20,6 +20,7 @@
patch_entry_return_type,
patch_reference_property,
)
+from lp.snappy.interfaces.hassnaps import IHasSnaps
from lp.snappy.interfaces.snap import (
ISnap,
ISnapSet,
@@ -31,6 +32,9 @@
)
+# IHasSnaps
+patch_collection_property(IHasSnaps, 'snaps', ISnap)
+
# ISnapFile
patch_reference_property(ISnapFile, 'snapbuild', ISnapBuild)
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2015-08-03 13:16:48 +0000
+++ lib/lp/snappy/model/snap.py 2015-08-04 13:10:52 +0000
@@ -334,6 +334,14 @@
"""See `ISnapSet`."""
return IStore(Snap).find(Snap, Snap.owner == owner)
+ def findByBranch(self, branch):
+ """See `ISnapSet`."""
+ return IStore(Snap).find(Snap, Snap.branch == branch)
+
+ def findByGitRepository(self, repository):
+ """See `ISnapSet`."""
+ return IStore(Snap).find(Snap, Snap.git_repository == repository)
+
def empty_list(self):
"""See `ISnapSet`."""
return []
=== added file 'lib/lp/snappy/tests/test_hassnaps.py'
--- lib/lp/snappy/tests/test_hassnaps.py 1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/tests/test_hassnaps.py 2015-08-04 13:10:52 +0000
@@ -0,0 +1,52 @@
+# Copyright 2015 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for classes that implement IHasSnaps."""
+
+__metaclass__ = type
+
+from lp.services.features.testing import FeatureFixture
+from lp.snappy.interfaces.hassnaps import IHasSnaps
+from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestIHasSnaps(TestCaseWithFactory):
+ """Test that the correct objects implement the interface."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestIHasSnaps, self).setUp()
+ self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+
+ def test_branch_implements_hasrecipes(self):
+ # Branches should implement IHasSnaps.
+ branch = self.factory.makeBranch()
+ self.assertProvides(branch, IHasSnaps)
+
+ def test_branch_recipes(self):
+ # IBranch.snaps should provide all the Snaps attached to that
+ # branch.
+ branch = self.factory.makeBranch()
+ self.factory.makeSnap(branch=branch)
+ self.factory.makeSnap(branch=branch)
+ self.factory.makeSnap()
+ self.assertEqual(2, branch.snaps.count())
+
+ def test_git_repository_implements_hasrecipes(self):
+ # Git repositories should implement IHasSnaps.
+ repository = self.factory.makeGitRepository()
+ self.assertProvides(repository, IHasSnaps)
+
+ def test_git_repository_recipes(self):
+ # IGitRepository.snaps should provide all the Snaps attached to that
+ # repository.
+ repository = self.factory.makeGitRepository()
+ [ref1, ref2] = self.factory.makeGitRefs(
+ repository=repository, paths=[u"refs/heads/1", u"refs/heads/2"])
+ self.factory.makeSnap(git_ref=ref1)
+ self.factory.makeSnap(git_ref=ref2)
+ self.factory.makeSnap()
+ self.assertEqual(2, repository.snaps.count())
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2015-08-03 13:16:48 +0000
+++ lib/lp/snappy/tests/test_snap.py 2015-08-04 13:10:52 +0000
@@ -451,6 +451,34 @@
self.assertContentEqual(
snaps[2:], getUtility(ISnapSet).findByPerson(owners[1]))
+ def test_findByBranch(self):
+ # ISnapSet.findByBranch returns all Snaps with the given Bazaar branch.
+ branches = [self.factory.makeAnyBranch() for i in range(2)]
+ snaps = []
+ for branch in branches:
+ for i in range(2):
+ snaps.append(self.factory.makeSnap(branch=branch))
+ self.assertContentEqual(
+ snaps[:2], getUtility(ISnapSet).findByBranch(branches[0]))
+ self.assertContentEqual(
+ snaps[2:], getUtility(ISnapSet).findByBranch(branches[1]))
+
+ def test_findByGitRepository(self):
+ # ISnapSet.findByGitRepository returns all Snaps with the given Git
+ # repository.
+ repositories = [self.factory.makeGitRepository() for i in range(2)]
+ snaps = []
+ for repository in repositories:
+ for i in range(2):
+ [ref] = self.factory.makeGitRefs(repository=repository)
+ snaps.append(self.factory.makeSnap(git_ref=ref))
+ self.assertContentEqual(
+ snaps[:2],
+ getUtility(ISnapSet).findByGitRepository(repositories[0]))
+ self.assertContentEqual(
+ snaps[2:],
+ getUtility(ISnapSet).findByGitRepository(repositories[1]))
+
class TestSnapProcessors(TestCaseWithFactory):
Follow ups
