launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18355
[Merge] lp:~cjwatson/launchpad/git-mp-collection into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-mp-collection into lp:launchpad with lp:~cjwatson/launchpad/git-mp-create as a prerequisite.
Commit message:
Add merge proposal support to GitCollection.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1445017 in Launchpad itself: "Support for Launchpad Git merge proposals "
https://bugs.launchpad.net/launchpad/+bug/1445017
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-mp-collection/+merge/257145
Add merge proposal support to GitCollection. This will be needed to support various views shortly.
The non-trivial database handling in GitCollection consists almost entirely of undoing simplifications I applied to it relative to BranchCollection when we didn't need merge proposal support. Now that we do, we need all the asymmetric filter stuff as well.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-mp-collection into lp:launchpad.
=== modified file 'lib/lp/code/interfaces/gitcollection.py'
--- lib/lp/code/interfaces/gitcollection.py 2015-04-15 18:34:25 +0000
+++ lib/lp/code/interfaces/gitcollection.py 2015-04-22 16:49:36 +0000
@@ -69,6 +69,42 @@
def getRepositoryIds():
"""Return a result set of all repository ids in this collection."""
+ # XXX cjwatson 2015-04-16: Add something like for_repositories or
+ # for_refs once we know exactly what we need.
+ def getMergeProposals(statuses=None, target_repository=None,
+ target_path=None, eager_load=False):
+ """Return a result set of merge proposals for the repositories in
+ this collection.
+
+ :param statuses: If specified, only return merge proposals with these
+ statuses. If not, return all merge proposals.
+ :param target_repository: If specified, only return merge proposals
+ that target the specified repository.
+ :param target_path: If specified, only return merge proposals that
+ target the specified path.
+ :param eager_load: If True, preloads all the related information for
+ merge proposals like PreviewDiffs and GitRepositories.
+ """
+
+ def getMergeProposalsForPerson(person, status=None):
+ """Proposals for `person`.
+
+ Return the proposals for repositories owned by `person` or where
+ `person` is reviewing or been asked to review.
+ """
+
+ def getMergeProposalsForReviewer(reviewer, status=None):
+ """Return a result set of merge proposals for the given reviewer.
+
+ That is, all merge proposals that 'reviewer' has voted on or has
+ been invited to vote on.
+
+ :param reviewer: An `IPerson` who is a reviewer.
+ :param status: An iterable of queue_status of the proposals to
+ return. If None is specified, all the proposals of all possible
+ states are returned.
+ """
+
def getTeamsWithRepositories(person):
"""Return the teams that person is a member of that have
repositories."""
@@ -117,6 +153,15 @@
"""Restrict the collection to repositories subscribed to by
'person'."""
+ def targetedBy(person, since=None):
+ """Restrict the collection to repositories targeted by person.
+
+ A repository is targeted by a person if that person has registered a
+ merge proposal with a reference in that repository as the target.
+
+ :param since: If supplied, ignore merge proposals before this date.
+ """
+
def visibleByUser(person):
"""Restrict the collection to repositories that person is allowed to
see."""
=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py 2015-04-22 16:49:35 +0000
+++ lib/lp/code/model/branchmergeproposal.py 2015-04-22 16:49:36 +0000
@@ -1054,16 +1054,19 @@
# Circular imports.
from lp.code.model.branch import Branch
from lp.code.model.branchcollection import GenericBranchCollection
+ from lp.code.model.gitcollection import GenericGitCollection
+ from lp.code.model.gitrepository import GitRepository
ids = set()
source_branch_ids = set()
+ source_git_repository_ids = set()
person_ids = set()
for mp in branch_merge_proposals:
ids.add(mp.id)
if mp.source_branchID is not None:
source_branch_ids.add(mp.source_branchID)
- # XXX cjwatson 2015-04-22: Implement for Git once GitCollection
- # supports MPs.
+ if mp.source_git_repositoryID is not None:
+ source_git_repository_ids.add(mp.source_git_repositoryID)
person_ids.add(mp.registrantID)
person_ids.add(mp.merge_reporterID)
@@ -1071,11 +1074,15 @@
Branch, branch_merge_proposals, (
"target_branchID", "prerequisite_branchID",
"source_branchID"))
+ repositories = load_related(
+ GitRepository, branch_merge_proposals, (
+ "target_git_repositoryID", "prerequisite_git_repositoryID",
+ "source_git_repositoryID"))
# The stacked on branches are used to check branch visibility.
GenericBranchCollection.preloadVisibleStackedOnBranches(
branches, user)
- if len(branches) == 0:
+ if len(branches) == 0 and len(repositories) == 0:
return
# Pre-load PreviewDiffs and Diffs.
@@ -1090,17 +1097,22 @@
cache = get_property_cache(previewdiff.branch_merge_proposal)
cache.preview_diff = previewdiff
- # Add source branch owners' to the list of pre-loaded persons.
+ # Add source branch/repository owners' to the list of pre-loaded
+ # persons.
person_ids.update(
branch.ownerID for branch in branches
if branch.id in source_branch_ids)
+ person_ids.update(
+ repository.owner_id for repository in repositories
+ if repository.id in source_git_repository_ids)
# Pre-load Person and ValidPersonCache.
list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
person_ids, need_validity=True))
- # Pre-load branches' data.
+ # Pre-load branches'/repositories' data.
GenericBranchCollection.preloadDataForBranches(branches)
+ GenericGitCollection.preloadDataForRepositories(repositories)
class BranchMergeProposalGetter:
=== modified file 'lib/lp/code/model/gitcollection.py'
--- lib/lp/code/model/gitcollection.py 2015-04-15 18:34:25 +0000
+++ lib/lp/code/model/gitcollection.py 2015-04-22 16:49:36 +0000
@@ -8,16 +8,25 @@
'GenericGitCollection',
]
+from functools import partial
+
from lazr.uri import (
InvalidURIError,
URI,
)
from storm.expr import (
+ And,
Count,
+ Desc,
In,
Join,
+ LeftJoin,
Select,
+ SQL,
+ With,
)
+from storm.info import ClassAlias
+from storm.store import EmptyResultSet
from zope.component import getUtility
from zope.interface import implements
@@ -31,14 +40,19 @@
from lp.code.interfaces.gitrepository import (
user_has_special_git_repository_access,
)
+from lp.code.model.branchmergeproposal import BranchMergeProposal
+from lp.code.model.codereviewcomment import CodeReviewComment
+from lp.code.model.codereviewvote import CodeReviewVoteReference
from lp.code.model.gitrepository import (
GitRepository,
get_git_repository_privacy_filter,
)
from lp.code.model.gitsubscription import GitSubscription
from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
+from lp.registry.model.distribution import Distribution
from lp.registry.model.person import Person
from lp.registry.model.product import Product
+from lp.registry.model.sourcepackagename import SourcePackageName
from lp.registry.model.teammembership import TeamParticipation
from lp.services.database.bulk import load_related
from lp.services.database.decoratedresultset import DecoratedResultSet
@@ -51,7 +65,8 @@
implements(IGitCollection)
- def __init__(self, store=None, filter_expressions=None, tables=None):
+ def __init__(self, store=None, filter_expressions=None, tables=None,
+ asymmetric_filter_expressions=None, asymmetric_tables=None):
"""Construct a `GenericGitCollection`.
:param store: The store to look in for repositories. If not
@@ -63,6 +78,10 @@
:param tables: A dict of Storm tables to the Join expression. If an
expression in filter_expressions refers to a table, then that
table *must* be in this list.
+ :param asymmetric_filter_expressions: As per filter_expressions but
+ only applies to one side of reflexive joins.
+ :param asymmetric_tables: As per tables, for
+ asymmetric_filter_expressions.
"""
self._store = store
if filter_expressions is None:
@@ -71,6 +90,13 @@
if tables is None:
tables = {}
self._tables = tables
+ if asymmetric_filter_expressions is None:
+ asymmetric_filter_expressions = []
+ self._asymmetric_filter_expressions = list(
+ asymmetric_filter_expressions)
+ if asymmetric_tables is None:
+ asymmetric_tables = {}
+ self._asymmetric_tables = asymmetric_tables
self._user = None
def count(self):
@@ -102,8 +128,13 @@
else:
return self._store
- def _filterBy(self, expressions, table=None, join=None):
- """Return a subset of this collection, filtered by 'expressions'."""
+ def _filterBy(self, expressions, table=None, join=None, symmetric=True):
+ """Return a subset of this collection, filtered by 'expressions'.
+
+ :param symmetric: If True, this filter will apply to both sides of
+ merge proposal lookups and any other lookups that join
+ GitRepository back onto GitRepository.
+ """
# NOTE: JonathanLange 2009-02-17: We might be able to avoid the need
# for explicit 'tables' by harnessing Storm's table inference system.
# See http://paste.ubuntu.com/118711/ for one way to do that.
@@ -112,10 +143,23 @@
if expressions is None:
expressions = []
tables = self._tables.copy()
+ asymmetric_tables = self._asymmetric_tables.copy()
+ if symmetric:
+ if table is not None:
+ tables[table] = join
+ symmetric_expr = self._filter_expressions + expressions
+ asymmetric_expr = list(self._asymmetric_filter_expressions)
+ else:
+ if table is not None:
+ asymmetric_tables[table] = join
+ symmetric_expr = list(self._filter_expressions)
+ asymmetric_expr = (
+ self._asymmetric_filter_expressions + expressions)
if table is not None:
tables[table] = join
return self.__class__(
- self.store, self._filter_expressions + expressions, tables)
+ self.store, symmetric_expr, tables,
+ asymmetric_expr, asymmetric_tables)
def _getRepositorySelect(self, columns=(GitRepository.id,)):
"""Return a Storm 'Select' for columns in this collection."""
@@ -126,15 +170,25 @@
def _getRepositoryExpressions(self):
"""Return the where expressions for this collection."""
return (self._filter_expressions +
+ self._asymmetric_filter_expressions +
self._getRepositoryVisibilityExpression())
- def _getRepositoryVisibilityExpression(self):
+ def _getRepositoryVisibilityExpression(self, repository_class=None):
"""Return the where clauses for visibility."""
return []
+ @staticmethod
+ def preloadDataForRepositories(repositories):
+ """Preload repositories' cached associated targets."""
+ load_related(Distribution, repositories, ['distribution_id'])
+ load_related(SourcePackageName, repositories, ['sourcepackagename_id'])
+ load_related(Product, repositories, ['project_id'])
+
def getRepositories(self, find_expr=GitRepository, eager_load=False):
"""See `IGitCollection`."""
- tables = [GitRepository] + list(set(self._tables.values()))
+ all_tables = set(
+ self._tables.values() + self._asymmetric_tables.values())
+ tables = [GitRepository] + list(all_tables)
expressions = self._getRepositoryExpressions()
resultset = self.store.using(*tables).find(find_expr, *expressions)
@@ -165,6 +219,148 @@
return self.getRepositories(
find_expr=GitRepository.id).get_plain_result_set()
+ def getMergeProposals(self, statuses=None, target_repository=None,
+ target_path=None, merged_revision_ids=None,
+ eager_load=False):
+ """See `IGitCollection`."""
+ if merged_revision_ids is not None and not merged_revision_ids:
+ # We have an empty revision list, so we can shortcut.
+ return EmptyResultSet()
+ elif (self._asymmetric_filter_expressions or
+ target_repository is not None or
+ target_path is not None or
+ merged_revision_ids is not None):
+ return self._naiveGetMergeProposals(
+ statuses, target_repository, target_path, merged_revision_ids,
+ eager_load=eager_load)
+ else:
+ # When examining merge proposals in a scope, this is a moderately
+ # effective set of constrained queries. It is not effective when
+ # unscoped or when tight constraints on repositories are present.
+ return self._scopedGetMergeProposals(
+ statuses, eager_load=eager_load)
+
+ def _naiveGetMergeProposals(self, statuses=None, target_repository=None,
+ target_path=None, merged_revision_ids=None,
+ eager_load=False):
+ Target = ClassAlias(GitRepository, "target")
+ extra_tables = list(set(
+ self._tables.values() + self._asymmetric_tables.values()))
+ tables = [GitRepository] + extra_tables + [
+ Join(BranchMergeProposal, And(
+ GitRepository.id ==
+ BranchMergeProposal.source_git_repositoryID,
+ *(self._filter_expressions +
+ self._asymmetric_filter_expressions))),
+ Join(Target,
+ Target.id == BranchMergeProposal.target_git_repositoryID),
+ ]
+ expressions = self._getRepositoryVisibilityExpression()
+ expressions.extend(self._getRepositoryVisibilityExpression(Target))
+ if target_repository is not None:
+ expressions.append(
+ BranchMergeProposal.target_git_repository == target_repository)
+ if target_path is not None:
+ expressions.append(
+ BranchMergeProposal.target_git_path == target_path)
+ if merged_revision_ids is not None:
+ expressions.append(
+ BranchMergeProposal.merged_revision_id.is_in(
+ merged_revision_ids))
+ if statuses is not None:
+ expressions.append(
+ BranchMergeProposal.queue_status.is_in(statuses))
+ resultset = self.store.using(*tables).find(
+ BranchMergeProposal, *expressions)
+ if not eager_load:
+ return resultset
+ else:
+ loader = partial(
+ BranchMergeProposal.preloadDataForBMPs, user=self._user)
+ return DecoratedResultSet(resultset, pre_iter_hook=loader)
+
+ def _scopedGetMergeProposals(self, statuses, eager_load=False):
+ expressions = (
+ self._filter_expressions
+ + self._getRepositoryVisibilityExpression())
+ with_expr = With(
+ "candidate_repositories",
+ Select(
+ GitRepository.id,
+ tables=[GitRepository] + self._tables.values(),
+ where=And(*expressions) if expressions else True))
+ expressions = [SQL("""
+ source_git_repository IN
+ (SELECT id FROM candidate_repositories) AND
+ target_git_repository IN
+ (SELECT id FROM candidate_repositories)""")]
+ tables = [BranchMergeProposal]
+ if self._asymmetric_filter_expressions:
+ # Need to filter on GitRepository beyond the with constraints.
+ expressions += self._asymmetric_filter_expressions
+ expressions.append(
+ BranchMergeProposal.source_git_repositoryID ==
+ GitRepository.id)
+ tables.append(GitRepository)
+ tables.extend(self._asymmetric_tables.values())
+ if statuses is not None:
+ expressions.append(
+ BranchMergeProposal.queue_status.is_in(statuses))
+ resultset = self.store.with_(with_expr).using(*tables).find(
+ BranchMergeProposal, *expressions)
+ if not eager_load:
+ return resultset
+ else:
+ loader = partial(
+ BranchMergeProposal.preloadDataForBMPs, user=self._user)
+ return DecoratedResultSet(resultset, pre_iter_hook=loader)
+
+ def getMergeProposalsForPerson(self, person, status=None,
+ eager_load=False):
+ """See `IGitCollection`."""
+ # We want to limit the proposals to those where the source repository
+ # is limited by the defined collection.
+ owned = self.ownedBy(person).getMergeProposals(status)
+ reviewing = self.getMergeProposalsForReviewer(person, status)
+ resultset = owned.union(reviewing)
+
+ if not eager_load:
+ return resultset
+ else:
+ loader = partial(
+ BranchMergeProposal.preloadDataForBMPs, user=self._user)
+ return DecoratedResultSet(resultset, pre_iter_hook=loader)
+
+ def getMergeProposalsForReviewer(self, reviewer, status=None):
+ """See `IGitCollection`."""
+ tables = [
+ BranchMergeProposal,
+ Join(CodeReviewVoteReference,
+ CodeReviewVoteReference.branch_merge_proposalID == \
+ BranchMergeProposal.id),
+ LeftJoin(CodeReviewComment,
+ CodeReviewVoteReference.commentID == CodeReviewComment.id)]
+
+ expressions = [
+ CodeReviewVoteReference.reviewer == reviewer,
+ BranchMergeProposal.source_git_repositoryID.is_in(
+ self._getRepositorySelect())]
+ visibility = self._getRepositoryVisibilityExpression()
+ if visibility:
+ expressions.append(
+ BranchMergeProposal.target_git_repositoryID.is_in(
+ Select(GitRepository.id, visibility)))
+ if status is not None:
+ expressions.append(
+ BranchMergeProposal.queue_status.is_in(status))
+ proposals = self.store.using(*tables).find(
+ BranchMergeProposal, *expressions)
+ # Apply sorting here as we can't do it in the browser code. We need to
+ # think carefully about the best places to do this, but not here nor
+ # now.
+ proposals.order_by(Desc(CodeReviewComment.vote))
+ return proposals
+
def getTeamsWithRepositories(self, person):
"""See `IGitCollection`."""
# This method doesn't entirely fit with the intent of the
@@ -221,18 +417,20 @@
def ownedBy(self, person):
"""See `IGitCollection`."""
- return self._filterBy([GitRepository.owner == person])
+ return self._filterBy([GitRepository.owner == person], symmetric=False)
def ownedByTeamMember(self, person):
"""See `IGitCollection`."""
subquery = Select(
TeamParticipation.teamID,
where=TeamParticipation.personID == person.id)
- return self._filterBy([In(GitRepository.owner_id, subquery)])
+ return self._filterBy(
+ [In(GitRepository.owner_id, subquery)], symmetric=False)
def registeredBy(self, person):
"""See `IGitCollection`."""
- return self._filterBy([GitRepository.registrant == person])
+ return self._filterBy(
+ [GitRepository.registrant == person], symmetric=False)
def _getExactMatch(self, term):
# Look up the repository by its URL, which handles both shortcuts
@@ -274,7 +472,21 @@
[GitSubscription.person == person],
table=GitSubscription,
join=Join(GitSubscription,
- GitSubscription.repository == GitRepository.id))
+ GitSubscription.repository == GitRepository.id),
+ symmetric=False)
+
+ def targetedBy(self, person, since=None):
+ """See `IGitCollection`."""
+ clauses = [BranchMergeProposal.registrant == person]
+ if since is not None:
+ clauses.append(BranchMergeProposal.date_created >= since)
+ return self._filterBy(
+ clauses,
+ table=BranchMergeProposal,
+ join=Join(
+ BranchMergeProposal,
+ BranchMergeProposal.target_git_repository == GitRepository.id),
+ symmetric=False)
def visibleByUser(self, person):
"""See `IGitCollection`."""
@@ -283,33 +495,46 @@
return self
if person is None:
return AnonymousGitCollection(
- self._store, self._filter_expressions, self._tables)
+ self._store, self._filter_expressions, self._tables,
+ self._asymmetric_filter_expressions, self._asymmetric_tables)
return VisibleGitCollection(
- person, self._store, self._filter_expressions, self._tables)
+ person, self._store, self._filter_expressions, self._tables,
+ self._asymmetric_filter_expressions, self._asymmetric_tables)
def withIds(self, *repository_ids):
"""See `IGitCollection`."""
- return self._filterBy([GitRepository.id.is_in(repository_ids)])
+ return self._filterBy(
+ [GitRepository.id.is_in(repository_ids)], symmetric=False)
class AnonymousGitCollection(GenericGitCollection):
"""Repository collection that only shows public repositories."""
- def _getRepositoryVisibilityExpression(self):
+ def _getRepositoryVisibilityExpression(self,
+ repository_class=GitRepository):
"""Return the where clauses for visibility."""
- return get_git_repository_privacy_filter(None)
+ return get_git_repository_privacy_filter(
+ None, repository_class=repository_class)
class VisibleGitCollection(GenericGitCollection):
"""A repository collection that has special logic for visibility."""
- def __init__(self, user, store=None, filter_expressions=None, tables=None):
+ def __init__(self, user, store=None, filter_expressions=None, tables=None,
+ asymmetric_filter_expressions=None, asymmetric_tables=None):
super(VisibleGitCollection, self).__init__(
- store=store, filter_expressions=filter_expressions, tables=tables)
+ store=store, filter_expressions=filter_expressions, tables=tables,
+ asymmetric_filter_expressions=asymmetric_filter_expressions,
+ asymmetric_tables=asymmetric_tables)
self._user = user
- def _filterBy(self, expressions, table=None, join=None):
- """Return a subset of this collection, filtered by 'expressions'."""
+ def _filterBy(self, expressions, table=None, join=None, symmetric=True):
+ """Return a subset of this collection, filtered by 'expressions'.
+
+ :param symmetric: If True this filter will apply to both sides of
+ merge proposal lookups and any other lookups that join
+ GitRepository back onto GitRepository.
+ """
# NOTE: JonathanLange 2009-02-17: We might be able to avoid the need
# for explicit 'tables' by harnessing Storm's table inference system.
# See http://paste.ubuntu.com/118711/ for one way to do that.
@@ -318,14 +543,30 @@
if expressions is None:
expressions = []
tables = self._tables.copy()
- if table is not None:
- tables[table] = join
+ asymmetric_tables = self._asymmetric_tables.copy()
+ if symmetric:
+ if table is not None:
+ tables[table] = join
+ symmetric_expr = self._filter_expressions + expressions
+ asymmetric_expr = list(self._asymmetric_filter_expressions)
+ else:
+ if table is not None:
+ asymmetric_tables[table] = join
+ symmetric_expr = list(self._filter_expressions)
+ asymmetric_expr = self._asymmetric_filter_expressions + expressions
return self.__class__(
- self._user, self.store, self._filter_expressions + expressions)
-
- def _getRepositoryVisibilityExpression(self):
- """Return the where clauses for visibility."""
- return get_git_repository_privacy_filter(self._user)
+ self._user, self.store, symmetric_expr, tables,
+ asymmetric_expr, asymmetric_tables)
+
+ def _getRepositoryVisibilityExpression(self,
+ repository_class=GitRepository):
+ """Return the where clauses for visibility.
+
+ :param repository_class: The GitRepository class to use - permits
+ using ClassAliases.
+ """
+ return get_git_repository_privacy_filter(
+ self._user, repository_class=repository_class)
def visibleByUser(self, person):
"""See `IGitCollection`."""
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2015-04-22 16:49:35 +0000
+++ lib/lp/code/model/gitrepository.py 2015-04-22 16:49:36 +0000
@@ -850,8 +850,8 @@
return []
-def get_git_repository_privacy_filter(user):
- public_filter = GitRepository.information_type.is_in(
+def get_git_repository_privacy_filter(user, repository_class=GitRepository):
+ public_filter = repository_class.information_type.is_in(
PUBLIC_INFORMATION_TYPES)
if user is None:
@@ -859,7 +859,7 @@
artifact_grant_query = Coalesce(
ArrayIntersects(
- SQL("GitRepository.access_grants"),
+ SQL("%s.access_grants" % repository_class.__storm_table__),
Select(
ArrayAgg(TeamParticipation.teamID),
tables=TeamParticipation,
@@ -868,7 +868,7 @@
policy_grant_query = Coalesce(
ArrayIntersects(
- Array(SQL("GitRepository.access_policy")),
+ Array(SQL("%s.access_policy" % repository_class.__storm_table__)),
Select(
ArrayAgg(AccessPolicyGrant.policy_id),
tables=(AccessPolicyGrant,
=== modified file 'lib/lp/code/model/tests/test_gitcollection.py'
--- lib/lp/code/model/tests/test_gitcollection.py 2015-04-15 18:34:25 +0000
+++ lib/lp/code/model/tests/test_gitcollection.py 2015-04-22 16:49:36 +0000
@@ -5,7 +5,18 @@
__metaclass__ = type
+from datetime import (
+ datetime,
+ timedelta,
+ )
+from operator import attrgetter
+
+import pytz
from testtools.matchers import Equals
+from storm.store import (
+ EmptyResultSet,
+ Store,
+ )
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -13,6 +24,7 @@
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.interfaces.services import IService
from lp.code.enums import (
+ BranchMergeProposalStatus,
BranchSubscriptionDiffSize,
BranchSubscriptionNotificationLevel,
CodeReviewNotificationLevel,
@@ -42,7 +54,10 @@
StormStatementRecorder,
TestCaseWithFactory,
)
-from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.testing.matchers import HasQueryCount
@@ -406,6 +421,263 @@
collection = self.all_repositories.subscribedBy(subscriber)
self.assertEqual([repository], list(collection.getRepositories()))
+ def test_targetedBy(self):
+ # Only repositories that are merge targets are returned.
+ [target_ref] = self.factory.makeGitRefs()
+ registrant = self.factory.makePerson()
+ self.factory.makeBranchMergeProposalForGit(
+ target_ref=target_ref, registrant=registrant)
+ # And another not registered by registrant.
+ self.factory.makeBranchMergeProposalForGit()
+ collection = self.all_repositories.targetedBy(registrant)
+ self.assertEqual(
+ [target_ref.repository], list(collection.getRepositories()))
+
+ def test_targetedBy_since(self):
+ # Ignore proposals created before 'since'.
+ bmp = self.factory.makeBranchMergeProposalForGit()
+ date_created = self.factory.getUniqueDate()
+ removeSecurityProxy(bmp).date_created = date_created
+ registrant = bmp.registrant
+ repositories = self.all_repositories.targetedBy(
+ registrant, since=date_created)
+ self.assertEqual(
+ [bmp.target_git_repository], list(repositories.getRepositories()))
+ since = self.factory.getUniqueDate()
+ repositories = self.all_repositories.targetedBy(
+ registrant, since=since)
+ self.assertEqual([], list(repositories.getRepositories()))
+
+
+class TestBranchMergeProposals(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ TestCaseWithFactory.setUp(self)
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+ self.all_repositories = getUtility(IAllGitRepositories)
+
+ def test_empty_branch_merge_proposals(self):
+ proposals = self.all_repositories.getMergeProposals()
+ self.assertEqual([], list(proposals))
+
+ def test_empty_revisions_shortcut(self):
+ # If you explicitly pass an empty collection of revision numbers,
+ # the method shortcuts and gives you an empty result set. In this
+ # way, merged_revnos=None (the default) has a very different behaviour
+ # than merged_revnos=[]: the first is no restriction, while the second
+ # excludes everything.
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals(
+ merged_revision_ids=[])
+ self.assertEqual([], list(proposals))
+ self.assertIsInstance(proposals, EmptyResultSet)
+
+ def test_some_branch_merge_proposals(self):
+ mp = self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals()
+ self.assertEqual([mp], list(proposals))
+
+ def test_just_owned_branch_merge_proposals(self):
+ # If the collection only includes branches owned by a person, the
+ # getMergeProposals() will only return merge proposals for source
+ # branches that are owned by that person.
+ person = self.factory.makePerson()
+ project = self.factory.makeProduct()
+ [ref1] = self.factory.makeGitRefs(target=project, owner=person)
+ [ref2] = self.factory.makeGitRefs(target=project, owner=person)
+ [ref3] = self.factory.makeGitRefs(target=project)
+ self.factory.makeGitRefs(target=project)
+ [target] = self.factory.makeGitRefs(target=project)
+ mp1 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target, source_ref=ref1)
+ mp2 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target, source_ref=ref2)
+ self.factory.makeBranchMergeProposalForGit(
+ target_ref=target, source_ref=ref3)
+ collection = self.all_repositories.ownedBy(person)
+ proposals = collection.getMergeProposals()
+ self.assertEqual(sorted([mp1, mp2]), sorted(proposals))
+
+ def test_preloading_for_previewdiff(self):
+ project = self.factory.makeProduct()
+ [target] = self.factory.makeGitRefs(target=project)
+ owner = self.factory.makePerson()
+ [ref1] = self.factory.makeGitRefs(target=project, owner=owner)
+ [ref2] = self.factory.makeGitRefs(target=project, owner=owner)
+ bmp1 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target, source_ref=ref1)
+ bmp2 = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target, source_ref=ref2)
+ old_date = datetime.now(pytz.UTC) - timedelta(hours=1)
+ self.factory.makePreviewDiff(
+ merge_proposal=bmp1, date_created=old_date)
+ previewdiff1 = self.factory.makePreviewDiff(merge_proposal=bmp1)
+ self.factory.makePreviewDiff(
+ merge_proposal=bmp2, date_created=old_date)
+ previewdiff2 = self.factory.makePreviewDiff(merge_proposal=bmp2)
+ Store.of(bmp1).flush()
+ Store.of(bmp1).invalidate()
+ collection = self.all_repositories.ownedBy(owner)
+ [pre_bmp1, pre_bmp2] = sorted(
+ collection.getMergeProposals(eager_load=True),
+ key=attrgetter('id'))
+ with StormStatementRecorder() as recorder:
+ self.assertEqual(
+ removeSecurityProxy(pre_bmp1.preview_diff).id, previewdiff1.id)
+ self.assertEqual(
+ removeSecurityProxy(pre_bmp2.preview_diff).id, previewdiff2.id)
+ self.assertThat(recorder, HasQueryCount(Equals(0)))
+
+ def test_merge_proposals_in_project(self):
+ mp1 = self.factory.makeBranchMergeProposalForGit()
+ self.factory.makeBranchMergeProposalForGit()
+ project = mp1.source_git_ref.target
+ collection = self.all_repositories.inProject(project)
+ proposals = collection.getMergeProposals()
+ self.assertEqual([mp1], list(proposals))
+
+ def test_target_branch_private(self):
+ # The target branch must be in the branch collection, as must the
+ # source branch.
+ registrant = self.factory.makePerson()
+ mp1 = self.factory.makeBranchMergeProposalForGit(registrant=registrant)
+ naked_repository = removeSecurityProxy(mp1.target_git_repository)
+ naked_repository.transitionToInformationType(
+ InformationType.USERDATA, registrant, verify_policy=False)
+ collection = self.all_repositories.visibleByUser(None)
+ proposals = collection.getMergeProposals()
+ self.assertEqual([], list(proposals))
+
+ def test_status_restriction(self):
+ mp1 = self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.WORK_IN_PROGRESS)
+ mp2 = self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+ self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.CODE_APPROVED)
+ proposals = self.all_repositories.getMergeProposals(
+ [BranchMergeProposalStatus.WORK_IN_PROGRESS,
+ BranchMergeProposalStatus.NEEDS_REVIEW])
+ self.assertEqual(sorted([mp1, mp2]), sorted(proposals))
+
+ def test_status_restriction_with_project_filter(self):
+ # getMergeProposals returns the merge proposals with a particular
+ # status that are _inside_ the repository collection. mp1 is in the
+ # product with NEEDS_REVIEW, mp2 is outside of the project and mp3
+ # has an excluded status.
+ mp1 = self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+ self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+ project = mp1.source_git_ref.target
+ [ref1] = self.factory.makeGitRefs(target=project)
+ [ref2] = self.factory.makeGitRefs(target=project)
+ self.factory.makeBranchMergeProposalForGit(
+ target_ref=ref1, source_ref=ref2,
+ set_state=BranchMergeProposalStatus.CODE_APPROVED)
+ collection = self.all_repositories.inProject(project)
+ proposals = collection.getMergeProposals(
+ [BranchMergeProposalStatus.NEEDS_REVIEW])
+ self.assertEqual([mp1], list(proposals))
+
+ def test_specifying_target_repository(self):
+ # If the target_repository is specified but not the target_path,
+ # only merge proposals where that repository is the target are
+ # returned.
+ [ref1, ref2] = self.factory.makeGitRefs(
+ paths=[u"refs/heads/ref1", u"refs/heads/ref2"])
+ mp1 = self.factory.makeBranchMergeProposalForGit(target_ref=ref1)
+ mp2 = self.factory.makeBranchMergeProposalForGit(target_ref=ref2)
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals(
+ target_repository=mp1.target_git_repository)
+ self.assertEqual(sorted([mp1, mp2]), sorted(proposals))
+
+ def test_specifying_target_ref(self):
+ # If the target_repository and target_path are specified, only merge
+ # proposals where that ref is the target are returned.
+ mp1 = self.factory.makeBranchMergeProposalForGit()
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals(
+ target_repository=mp1.target_git_repository,
+ target_path=mp1.target_git_path)
+ self.assertEqual([mp1], list(proposals))
+
+
+class TestBranchMergeProposalsForReviewer(TestCaseWithFactory):
+ """Tests for IGitCollection.getProposalsForReviewer()."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ # Use the admin user as we don't care about who can and can't call
+ # nominate reviewer in this test.
+ TestCaseWithFactory.setUp(self, 'admin@xxxxxxxxxxxxx')
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+ self.all_repositories = getUtility(IAllGitRepositories)
+
+ def test_getProposalsForReviewer(self):
+ reviewer = self.factory.makePerson()
+ proposal = self.factory.makeBranchMergeProposalForGit()
+ proposal.nominateReviewer(reviewer, reviewer)
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposalsForReviewer(
+ reviewer)
+ self.assertEqual([proposal], list(proposals))
+
+ def test_getProposalsForReviewer_filter_status(self):
+ reviewer = self.factory.makePerson()
+ proposal1 = self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+ proposal1.nominateReviewer(reviewer, reviewer)
+ proposal2 = self.factory.makeBranchMergeProposalForGit(
+ set_state=BranchMergeProposalStatus.WORK_IN_PROGRESS)
+ proposal2.nominateReviewer(reviewer, reviewer)
+ proposals = self.all_repositories.getMergeProposalsForReviewer(
+ reviewer, [BranchMergeProposalStatus.NEEDS_REVIEW])
+ self.assertEqual([proposal1], list(proposals))
+
+ def test_getProposalsForReviewer_anonymous(self):
+ # Don't include proposals if the target branch is private for
+ # anonymous views.
+ reviewer = self.factory.makePerson()
+ [target_ref] = self.factory.makeGitRefs(
+ information_type=InformationType.USERDATA)
+ proposal = self.factory.makeBranchMergeProposalForGit(
+ target_ref=target_ref)
+ proposal.nominateReviewer(reviewer, reviewer)
+ proposals = self.all_repositories.visibleByUser(
+ None).getMergeProposalsForReviewer(reviewer)
+ self.assertEqual([], list(proposals))
+
+ def test_getProposalsForReviewer_anonymous_source_private(self):
+ # Don't include proposals if the source branch is private for
+ # anonymous views.
+ reviewer = self.factory.makePerson()
+ project = self.factory.makeProduct()
+ [source_ref] = self.factory.makeGitRefs(
+ target=project, information_type=InformationType.USERDATA)
+ [target_ref] = self.factory.makeGitRefs(target=project)
+ proposal = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source_ref, target_ref=target_ref)
+ proposal.nominateReviewer(reviewer, reviewer)
+ proposals = self.all_repositories.visibleByUser(
+ None).getMergeProposalsForReviewer(reviewer)
+ self.assertEqual([], list(proposals))
+
+ def test_getProposalsForReviewer_for_product(self):
+ reviewer = self.factory.makePerson()
+ proposal = self.factory.makeBranchMergeProposalForGit()
+ proposal.nominateReviewer(reviewer, reviewer)
+ proposal2 = self.factory.makeBranchMergeProposalForGit()
+ proposal2.nominateReviewer(reviewer, reviewer)
+ proposals = self.all_repositories.inProject(
+ proposal.merge_source.target).getMergeProposalsForReviewer(
+ reviewer)
+ self.assertEqual([proposal], list(proposals))
+
class TestGenericGitCollectionVisibleFilter(TestCaseWithFactory):
Follow ups
