Merge lp:~cjwatson/launchpad/git-mp-collection into lp:launchpad

[Colin Watson]

Add merge proposal support to GitCollection.

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.

[William Grant (wgrant)]

review: Approve (code)

=== 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-24 11:37:11 +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 14:52:10 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2015-04-24 11:37:11 +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-24 11:37:11 +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:11:40 +0000
+++ lib/lp/code/model/gitrepository.py	2015-04-24 11:37:11 +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-24 11:37:11 +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, '[e-mail]')
+        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):