← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/bmp-buglinktarget-git into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/bmp-buglinktarget-git into lp:launchpad with lp:~cjwatson/launchpad/bmp-buglinktarget as a prerequisite.

Commit message:
Implement the basics of bug linking for Git-based merge proposals.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1492926 in Launchpad itself: "Git merge proposals can't be linked to bugs"
  https://bugs.launchpad.net/launchpad/+bug/1492926

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bmp-buglinktarget-git/+merge/298366

Implement the basics of bug linking for Git-based merge proposals.

These are stored as XRefs rather than using another link table.  As yet there's no exported way to create these links; the only externally-visible effect of this MP is that the webservice gains bug.linked_merge_proposals.

Note that bug.linked_branches and bug.linked_merge_proposals are currently separate (i.e. the latter doesn't magically proxy for the former in the Bazaar case), and any clients would need to query both.  I'm open to discussion about this but it definitely simplified the implementation, and I suspect that not too many clients need to query this (for example, Tarmac and similar landing robots typically work in the other link direction).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bmp-buglinktarget-git into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-06-14 12:40:30 +0000
+++ database/schema/security.cfg	2016-06-25 09:46:10 +0000
@@ -718,6 +718,7 @@
 public.validpersonorteamcache             = SELECT
 public.webhook                            = SELECT
 public.webhookjob                         = SELECT, INSERT
+public.xref                               = SELECT, INSERT, DELETE
 type=user
 
 [branch-distro]
@@ -2007,6 +2008,7 @@
 public.validpersoncache                 = SELECT
 public.webhook                          = SELECT
 public.webhookjob                       = SELECT, INSERT
+public.xref                             = SELECT
 type=user
 
 [sharing-jobs]
@@ -2082,6 +2084,7 @@
 public.teammembership                   = SELECT
 public.teamparticipation                = SELECT
 public.validpersoncache                 = SELECT
+public.xref                             = SELECT
 type=user
 
 [reclaim-branch-space]

=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py	2015-10-26 14:54:43 +0000
+++ lib/lp/_schema_circular_imports.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Update the interface schema values due to circular imports.
@@ -681,6 +681,7 @@
     IBug, 'getNominationFor', 'target', IBugTarget)
 patch_plain_parameter_type(
     IBug, 'getNominations', 'target', IBugTarget)
+patch_collection_property(IBug, 'linked_merge_proposals', IBranchMergeProposal)
 patch_choice_parameter_type(
     IBug, 'subscribe', 'level', BugNotificationLevel)
 

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2016-02-04 12:45:38 +0000
+++ lib/lp/bugs/configure.zcml	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -707,7 +707,9 @@
                 attributes="
                     linked_branches
                     linked_bugbranches
-                    getVisibleLinkedBranches"/>
+                    getVisibleLinkedBranches
+                    linked_merge_proposals
+                    getVisibleLinkedMergeProposals"/>
             <require
                 permission="launchpad.Edit"
                 interface="lp.bugs.interfaces.bug.IBugEdit"

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2016-02-04 12:21:45 +0000
+++ lib/lp/bugs/interfaces/bug.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces related to bugs."""
@@ -809,6 +809,13 @@
         file_alias.restricted.
         """
 
+    def linkMergeProposal(merge_proposal, user, check_permissions=True):
+        """Ensure that this MP is linked to this bug."""
+
+    def unlinkMergeProposal(merge_proposal, user, check_permissions=True):
+        """Ensure that any links between this bug and the given MP are removed.
+        """
+
     @call_with(user=REQUEST_USER)
     @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True))
     @export_write_operation()
@@ -1009,8 +1016,23 @@
     @export_read_operation()
     @operation_for_version('beta')
     def getVisibleLinkedBranches(user):
-        """Return the branches linked to this bug that are visible by
-        `user`."""
+        """Return all the branches linked to the bug that `user` can see."""
+
+    linked_merge_proposals = exported(
+        CollectionField(
+            title=_("Merge proposals associated with this bug."),
+            # Really IBranchMergeProposal, patched in
+            # _schema_circular_imports.py.
+            value_type=Reference(schema=Interface),
+            readonly=True),
+        as_of='devel')
+
+    @accessor_for(linked_merge_proposals)
+    @call_with(user=REQUEST_USER)
+    @export_read_operation()
+    @operation_for_version('devel')
+    def getVisibleLinkedMergeProposals(user):
+        """Return all the MPs linked to the bug that `user` can see."""
 
 
 # We are forced to define these now to avoid circular import problems.

=== modified file 'lib/lp/bugs/interfaces/bugtasksearch.py'
--- lib/lp/bugs/interfaces/bugtasksearch.py	2015-10-15 14:09:50 +0000
+++ lib/lp/bugs/interfaces/bugtasksearch.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for searching bug tasks. Mostly used with IBugTaskSet."""
@@ -173,7 +173,8 @@
                  hardware_owner_is_affected_by_bug=False,
                  hardware_owner_is_subscribed_to_bug=False,
                  hardware_is_linked_to_bug=False,
-                 linked_branches=None, linked_blueprints=None,
+                 linked_branches=None, linked_merge_proposals=None,
+                 linked_blueprints=None,
                  structural_subscriber=None, modified_since=None,
                  created_since=None, exclude_conjoined_tasks=False, cve=None,
                  upstream_target=None, milestone_dateexpected_before=None,
@@ -222,6 +223,7 @@
             hardware_owner_is_subscribed_to_bug)
         self.hardware_is_linked_to_bug = hardware_is_linked_to_bug
         self.linked_branches = linked_branches
+        self.linked_merge_proposals = linked_merge_proposals
         self.linked_blueprints = linked_blueprints
         self.structural_subscriber = structural_subscriber
         self.modified_since = modified_since
@@ -382,7 +384,8 @@
                        hardware_owner_is_affected_by_bug=False,
                        hardware_owner_is_subscribed_to_bug=False,
                        hardware_is_linked_to_bug=False, linked_branches=None,
-                       linked_blueprints=None, structural_subscriber=None,
+                       linked_merge_proposals=None, linked_blueprints=None,
+                       structural_subscriber=None,
                        modified_since=None, created_since=None,
                        created_before=None, information_type=None):
         """Create and return a new instance using the parameter list."""
@@ -451,6 +454,7 @@
         search_params.hardware_is_linked_to_bug = (
             hardware_is_linked_to_bug)
         search_params.linked_branches = linked_branches
+        search_params.linked_merge_proposals = linked_merge_proposals
         search_params.linked_blueprints = linked_blueprints
         search_params.structural_subscriber = structural_subscriber
         search_params.modified_since = modified_since

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2016-02-04 12:54:05 +0000
+++ lib/lp/bugs/model/bug.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Launchpad bug-related database table classes."""
@@ -166,6 +166,7 @@
     get_structural_subscriptions,
     )
 from lp.code.interfaces.branchcollection import IAllBranches
+from lp.code.interfaces.gitcollection import IAllGitRepositories
 from lp.hardwaredb.interfaces.hwdb import IHWSubmissionBugSet
 from lp.registry.errors import CannotChangeInformationType
 from lp.registry.interfaces.accesspolicy import (
@@ -1380,7 +1381,7 @@
             Store.of(bug_branch).remove(bug_branch)
 
     def getVisibleLinkedBranches(self, user, eager_load=False):
-        """Return all the branches linked to the bug that `user` can see."""
+        """See `IBug`."""
         linked_branches = list(getUtility(IAllBranches).visibleByUser(
             user).linkedToBugs([self]).getBranches(eager_load=eager_load))
         if len(linked_branches) == 0:
@@ -1391,6 +1392,39 @@
                 BugBranch,
                 BugBranch.bug == self, In(BugBranch.branchID, branch_ids))
 
+    def linkMergeProposal(self, merge_proposal, user, check_permissions=True):
+        """See `IBug`."""
+        merge_proposal.linkBug(
+            self, user=user, check_permissions=check_permissions)
+
+    def unlinkMergeProposal(self, merge_proposal, user,
+                            check_permissions=True):
+        """See `IBug`."""
+        merge_proposal.unlinkBug(
+            self, user=user, check_permissions=check_permissions)
+
+    @property
+    def linked_merge_proposals(self):
+        from lp.code.model.branchmergeproposal import BranchMergeProposal
+        merge_proposal_ids = [
+            int(id) for _, id in getUtility(IXRefSet).findFrom(
+                (u'bug', unicode(self.id)), types=[u'merge_proposal'])]
+        return list(sorted(
+            bulk.load(BranchMergeProposal, merge_proposal_ids),
+            key=operator.attrgetter('id')))
+
+    def getVisibleLinkedMergeProposals(self, user, eager_load=False):
+        """See `IBug`."""
+        linked_merge_proposal_ids = set(
+            bmp.id for bmp in self.linked_merge_proposals)
+        # XXX cjwatson 2016-06-24: This will also need to look at
+        # IAllBranches in the event that we start linking bugs directly to
+        # Bazaar-based merge proposals rather than to their source branches.
+        collection = getUtility(IAllGitRepositories).visibleByUser(user)
+        return collection.getMergeProposals(
+            merge_proposal_ids=linked_merge_proposal_ids,
+            eager_load=eager_load)
+
     @cachedproperty
     def has_cves(self):
         """See `IBug`."""

=== modified file 'lib/lp/bugs/model/bugtasksearch.py'
--- lib/lp/bugs/model/bugtasksearch.py	2015-09-28 12:36:14 +0000
+++ lib/lp/bugs/model/bugtasksearch.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -671,16 +671,38 @@
                     BugBranch.branchID, branches))
         return Exists(Select(1, tables=[BugBranch], where=And(*where)))
 
+    def make_merge_proposal_clause(merge_proposals=None):
+        where = [
+            XRef.from_type == u'bug',
+            XRef.from_id_int == BugTaskFlat.bug_id,
+            XRef.to_type == u'merge_proposal',
+            ]
+        if merge_proposals is not None:
+            where.append(
+                search_value_to_storm_where_condition(
+                    XRef.to_id_int, merge_proposals))
+        return Exists(Select(1, tables=[XRef], where=And(*where)))
+
     if zope_isinstance(params.linked_branches, BaseItem):
         if params.linked_branches == BugBranchSearch.BUGS_WITH_BRANCHES:
-            extra_clauses.append(make_branch_clause())
+            extra_clauses.append(
+                Or(make_branch_clause(), make_merge_proposal_clause()))
         elif (params.linked_branches ==
                 BugBranchSearch.BUGS_WITHOUT_BRANCHES):
             extra_clauses.append(Not(make_branch_clause()))
+            extra_clauses.append(Not(make_merge_proposal_clause()))
     elif zope_isinstance(params.linked_branches, (any, all, int)):
-        # A specific search term has been supplied.
+        # A specific search term has been supplied.  Note that this only
+        # works with branches, not merge proposals, as it takes integer
+        # branch IDs.
         extra_clauses.append(make_branch_clause(params.linked_branches))
 
+    if zope_isinstance(params.linked_merge_proposals, (any, all, int)):
+        # This is normally only used internally by
+        # BranchMergeProposal.getRelatedBugTasks.
+        extra_clauses.append(
+            make_merge_proposal_clause(params.linked_merge_proposals))
+
     linked_blueprints_clause = _build_blueprint_related_clause(params)
     if linked_blueprints_clause is not None:
         extra_clauses.append(linked_blueprints_clause)

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2015-10-13 16:58:20 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -521,6 +521,36 @@
         self.assertContentEqual(public_branches, linked_branches)
         self.assertNotIn(private_branch, linked_branches)
 
+    def test_getVisibleLinkedMergeProposals_doesnt_rtn_inaccessible_mps(self):
+        # If a Bug has merge proposals linked to it that the current user
+        # cannot access, those merge proposals will not be returned in its
+        # linked_merge_proposals property.
+        bug = self.factory.makeBug()
+        private_owner = self.factory.makePerson()
+        [private_git_ref] = self.factory.makeGitRefs(
+            owner=private_owner, information_type=InformationType.USERDATA)
+        private_bmp = self.factory.makeBranchMergeProposalForGit(
+            source_ref=private_git_ref)
+        with person_logged_in(private_owner):
+            bug.linkMergeProposal(private_bmp, private_bmp.registrant)
+        public_owner = self.factory.makePerson()
+        public_git_refs = [
+            self.factory.makeGitRefs()[0] for i in range(4)]
+        public_bmps = [
+            self.factory.makeBranchMergeProposalForGit(source_ref=git_ref)
+            for git_ref in public_git_refs]
+        with person_logged_in(public_owner):
+            for public_bmp in public_bmps:
+                bug.linkMergeProposal(public_bmp, public_bmp.registrant)
+        with StormStatementRecorder() as recorder:
+            linked_merge_proposals = list(
+                bug.getVisibleLinkedMergeProposals(user=public_owner))
+            # We check that the query count is low, since that's part of the
+            # point of the way that linked_merge_proposals is implemented.
+            self.assertThat(recorder, HasQueryCount(LessThan(7)))
+        self.assertContentEqual(public_bmps, linked_merge_proposals)
+        self.assertNotIn(private_bmp, linked_merge_proposals)
+
     def test_getDirectSubscribers_with_recipients_query_count(self):
         # getDirectSubscribers() uses a constant number of queries when given
         # a recipients argument regardless of the number of subscribers.

=== modified file 'lib/lp/bugs/model/tests/test_bugtasksearch.py'
--- lib/lp/bugs/model/tests/test_bugtasksearch.py	2015-09-28 12:36:14 +0000
+++ lib/lp/bugs/model/tests/test_bugtasksearch.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -393,16 +393,18 @@
 
     def test_branches_linked(self):
         # Search results can be limited to bugs with or without linked
-        # branches.
+        # branches and merge proposals.
         with person_logged_in(self.owner):
             branch = self.factory.makeBranch()
             self.bugtasks[0].bug.linkBranch(branch, self.owner)
+            bmp = self.factory.makeBranchMergeProposalForGit()
+            self.bugtasks[1].bug.linkMergeProposal(bmp, self.owner)
         params = self.getBugTaskSearchParams(
             user=None, linked_branches=BugBranchSearch.BUGS_WITH_BRANCHES)
-        self.assertSearchFinds(params, self.bugtasks[:1])
+        self.assertSearchFinds(params, self.bugtasks[:2])
         params = self.getBugTaskSearchParams(
             user=None, linked_branches=BugBranchSearch.BUGS_WITHOUT_BRANCHES)
-        self.assertSearchFinds(params, self.bugtasks[1:])
+        self.assertSearchFinds(params, self.bugtasks[2:])
 
     def test_blueprints_linked(self):
         # Search results can be limited to bugs with or without linked

=== modified file 'lib/lp/bugs/tests/buglinktarget.txt'
--- lib/lp/bugs/tests/buglinktarget.txt	2016-01-26 15:47:37 +0000
+++ lib/lp/bugs/tests/buglinktarget.txt	2016-06-25 09:46:10 +0000
@@ -3,7 +3,7 @@
 Launchpad includes Malone, the powerful bug tracker. One of the best
 features of Malone is the ability to track a bug in multiple products
 and/or packages. A bug can also be linked to other non-bug tracking
-objects like questions, CVEs or specifications.
+objects like questions, CVEs, specifications, or merge proposals.
 
 The IBugLinkTarget interface is used for that general purpose linking.
 This file documents that interface and can be used to validate

=== modified file 'lib/lp/bugs/tests/test_buglinktarget.py'
--- lib/lp/bugs/tests/test_buglinktarget.py	2012-06-06 16:04:34 +0000
+++ lib/lp/bugs/tests/test_buglinktarget.py	2016-06-25 09:46:10 +0000
@@ -1,10 +1,10 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test harness for running the buglinktarget.txt interface test
 
-This module will run the interface test against the CVE, Specification and
-Question implementations of that interface.
+This module will run the interface test against the CVE, Specification,
+Question, and BranchMergeProposal implementations of that interface.
 """
 
 __metaclass__ = type
@@ -14,10 +14,12 @@
 import unittest
 
 from zope.component import getUtility
+from zope.security.proxy import ProxyFactory
 
 from lp.answers.interfaces.questioncollection import IQuestionSet
 from lp.blueprints.interfaces.specification import ISpecificationSet
 from lp.bugs.interfaces.cve import ICveSet
+from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.layers import LaunchpadFunctionalLayer
 from lp.testing.systemdocs import (
     LayeredDocFileSuite,
@@ -42,12 +44,20 @@
         'http://wiki.mozilla.org/Firefox:1.1_Product_Team')
 
 
+def branchMergeProposalSetUp(test):
+    setUp(test)
+    factory = LaunchpadObjectFactory()
+    test.globs['target'] = ProxyFactory(
+        factory.makeBranchMergeProposalForGit())
+
+
 def test_suite():
     suite = unittest.TestSuite()
 
     targets = [('cve', cveSetUp),
                ('question', questionSetUp),
                ('specification', specificationSetUp),
+               ('branchmergeproposal', branchMergeProposalSetUp),
                ]
 
     for name, setUpMethod in targets:

=== modified file 'lib/lp/code/model/branchmergeproposal.py'
--- lib/lp/code/model/branchmergeproposal.py	2016-06-25 09:46:10 +0000
+++ lib/lp/code/model/branchmergeproposal.py	2016-06-25 09:46:10 +0000
@@ -43,6 +43,9 @@
 from zope.interface import implementer
 
 from lp.app.enums import PRIVATE_INFORMATION_TYPES
+from lp.bugs.interfaces.bugtask import IBugTaskSet
+from lp.bugs.interfaces.bugtaskfilter import filter_bugtasks_by_context
+from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
 from lp.bugs.model.buglinktarget import BugLinkTargetMixin
 from lp.code.enums import (
     BranchMergeProposalStatus,
@@ -111,6 +114,7 @@
     quote,
     SQLBase,
     )
+from lp.services.helpers import shortlist
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
 from lp.services.mail.sendmail import validate_message
@@ -118,6 +122,7 @@
     cachedproperty,
     get_property_cache,
     )
+from lp.services.xref.interfaces import IXRefSet
 
 
 def is_valid_transition(proposal, from_state, next_state, user=None):
@@ -358,12 +363,15 @@
 
     @property
     def bugs(self):
+        from lp.bugs.model.bug import Bug
         if self.source_branch is not None:
             # For Bazaar, we currently only store bug/branch links.
             bugs = self.source_branch.linked_bugs
         else:
-            # XXX cjwatson 2016-06-24: Implement for Git.
-            bugs = []
+            bug_ids = [
+                int(id) for _, id in getUtility(IXRefSet).findFrom(
+                    (u'merge_proposal', unicode(self.id)), types=[u'bug'])]
+            bugs = load(Bug, bug_ids)
         return list(sorted(bugs, key=attrgetter('id')))
 
     def getRelatedBugTasks(self, user):
@@ -377,8 +385,24 @@
             return [bugtask
                 for bugtask in source_tasks if bugtask not in target_tasks]
         else:
-            # XXX cjwatson 2016-06-24: Implement for Git.
-            return []
+            params = BugTaskSearchParams(
+                user=user, linked_merge_proposals=self.id)
+            tasks = shortlist(getUtility(IBugTaskSet).search(params), 1000)
+            return filter_bugtasks_by_context(
+                self.source_git_repository.target, tasks)
+
+    def createBugLink(self, bug):
+        """See `BugLinkTargetMixin`."""
+        # XXX cjwatson 2016-06-11: Should set creator.
+        getUtility(IXRefSet).create(
+            {(u'merge_proposal', unicode(self.id)):
+                {(u'bug', unicode(bug.id)): {}}})
+
+    def deleteBugLink(self, bug):
+        """See `BugLinkTargetMixin`."""
+        getUtility(IXRefSet).delete(
+            {(u'merge_proposal', unicode(self.id)):
+                [(u'bug', unicode(bug.id))]})
 
     def linkBug(self, bug, user=None, check_permissions=True):
         """See `BugLinkTargetMixin`."""
@@ -386,8 +410,9 @@
             # For Bazaar, we currently only store bug/branch links.
             return self.source_branch.linkBug(bug, user)
         else:
-            # XXX cjwatson 2016-06-24: Implement for Git.
-            raise NotImplementedError
+            # Otherwise, link the bug to the merge proposal directly.
+            return super(BranchMergeProposal, self).linkBug(
+                bug, user=user, check_permissions=check_permissions)
 
     def unlinkBug(self, bug, user=None, check_permissions=True):
         """See `BugLinkTargetMixin`."""
@@ -399,8 +424,9 @@
             # this would require a complicated data migration.
             return self.source_branch.unlinkBug(bug, user)
         else:
-            # XXX cjwatson 2016-06-24: Implement for Git.
-            raise NotImplementedError
+            # Otherwise, link the bug to the merge proposal directly.
+            return super(BranchMergeProposal, self).unlinkBug(
+                bug, user=user, check_permissions=check_permissions)
 
     @property
     def address(self):

=== modified file 'lib/lp/code/model/gitcollection.py'
--- lib/lp/code/model/gitcollection.py	2016-05-05 07:22:32 +0000
+++ lib/lp/code/model/gitcollection.py	2016-06-25 09:46:10 +0000
@@ -242,7 +242,7 @@
     def getMergeProposals(self, statuses=None, target_repository=None,
                           target_path=None, prerequisite_repository=None,
                           prerequisite_path=None, merged_revision_ids=None,
-                          eager_load=False):
+                          merge_proposal_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.
@@ -252,11 +252,12 @@
             target_path is not None or
             prerequisite_repository is not None or
             prerequisite_path is not None or
-            merged_revision_ids is not None):
+            merged_revision_ids is not None or
+            merge_proposal_ids is not None):
             return self._naiveGetMergeProposals(
                 statuses, target_repository, target_path,
                 prerequisite_repository, prerequisite_path,
-                merged_revision_ids, eager_load=eager_load)
+                merged_revision_ids, merge_proposal_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
@@ -267,7 +268,8 @@
     def _naiveGetMergeProposals(self, statuses=None, target_repository=None,
                                 target_path=None, prerequisite_repository=None,
                                 prerequisite_path=None,
-                                merged_revision_ids=None, eager_load=False):
+                                merged_revision_ids=None,
+                                merge_proposal_ids=None, eager_load=False):
         Target = ClassAlias(GitRepository, "target")
         extra_tables = list(set(
             self._tables.values() + self._asymmetric_tables.values()))
@@ -299,6 +301,9 @@
             expressions.append(
                 BranchMergeProposal.merged_revision_id.is_in(
                     merged_revision_ids))
+        if merge_proposal_ids is not None:
+            expressions.append(
+                BranchMergeProposal.id.is_in(merge_proposal_ids))
         if statuses is not None:
             expressions.append(
                 BranchMergeProposal.queue_status.is_in(statuses))

=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2016-06-25 09:46:10 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2016-06-25 09:46:10 +0000
@@ -1400,7 +1400,7 @@
 
     def test_related_bugtasks_includes_source_bugtasks(self):
         # related_bugtasks includes bugtasks linked to the source branch in
-        # the Bazaar case.
+        # the Bazaar case, or directly to the MP in the Git case.
         bmp = self._makeBranchMergeProposal()
         bug = self.factory.makeBug()
         bmp.linkBug(bug, bmp.registrant)
@@ -1448,6 +1448,22 @@
         self.assertEqual([], list(bmp.getRelatedBugTasks(self.user)))
 
 
+class TestBranchMergeProposalBugsGit(
+    TestBranchMergeProposalBugsMixin, TestCaseWithFactory):
+
+    def setUp(self):
+        super(TestBranchMergeProposalBugsGit, self).setUp()
+        # Disable GitRef._getLog's use of memcache; we don't need it here,
+        # it requires more time-consuming test setup, and it makes it harder
+        # to repeatedly run updateRelatedBugsFromSource with different log
+        # responses.
+        self.useFixture(FeatureFixture(
+            {u"code.git.log.disable_memcache": u"on"}))
+
+    def _makeBranchMergeProposal(self):
+        return self.factory.makeBranchMergeProposalForGit()
+
+
 class TestNotifyModified(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
@@ -2353,8 +2369,8 @@
             [mp], list(target.getMergeProposals(
                 status=['Merged'], merged_revnos=[123])))
 
-    def test_getRelatedBugTasks(self):
-        """Test the getRelatedBugTasks API."""
+    def test_getRelatedBugTasks_bzr(self):
+        """Test the getRelatedBugTasks API for Bazaar."""
         db_bmp = self.factory.makeBranchMergeProposal()
         db_bug = self.factory.makeBug()
         db_bmp.source_branch.linkBug(db_bug, db_bmp.registrant)
@@ -2363,6 +2379,16 @@
         bugtask = self.wsObject(db_bug.default_bugtask)
         self.assertEqual([bugtask], list(bmp.getRelatedBugTasks()))
 
+    def test_getRelatedBugTasks_git(self):
+        """Test the getRelatedBugTasks API for Git."""
+        db_bmp = self.factory.makeBranchMergeProposalForGit()
+        db_bug = self.factory.makeBug()
+        db_bmp.linkBug(db_bug, db_bmp.registrant)
+        transaction.commit()
+        bmp = self.wsObject(db_bmp)
+        bugtask = self.wsObject(db_bug.default_bugtask)
+        self.assertEqual([bugtask], list(bmp.getRelatedBugTasks()))
+
     def test_setStatus_invalid_transition(self):
         """Emit BadRequest when an invalid transition is requested."""
         bmp = self.factory.makeBranchMergeProposal()

=== modified file 'lib/lp/code/model/tests/test_gitcollection.py'
--- lib/lp/code/model/tests/test_gitcollection.py	2015-10-12 12:58:32 +0000
+++ lib/lp/code/model/tests/test_gitcollection.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for Git repository collections."""
@@ -531,6 +531,21 @@
         proposals = collection.getMergeProposals()
         self.assertEqual([mp1], list(proposals))
 
+    def test_merge_proposals_by_id(self):
+        # merge_proposal_ids limits the returned merge proposals by ID.
+        [target] = self.factory.makeGitRefs()
+        mp1 = self.factory.makeBranchMergeProposalForGit(target_ref=target)
+        mp2 = self.factory.makeBranchMergeProposalForGit(target_ref=target)
+        self.factory.makeBranchMergeProposalForGit(target_ref=target)
+        result = self.all_repositories.getMergeProposals(
+            target_repository=target.repository, target_path=target.path,
+            merge_proposal_ids=[mp1.id, mp2.id])
+        self.assertContentEqual([mp1, mp2], result)
+        result = self.all_repositories.getMergeProposals(
+            target_repository=target.repository, target_path=target.path,
+            merge_proposal_ids=[])
+        self.assertContentEqual([], result)
+
     def test_target_branch_private(self):
         # The target branch must be in the branch collection, as must the
         # source branch.

=== modified file 'lib/lp/services/xref/interfaces.py'
--- lib/lp/services/xref/interfaces.py	2015-09-23 07:55:55 +0000
+++ lib/lp/services/xref/interfaces.py	2016-06-25 09:46:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -28,8 +28,9 @@
 
         :param xrefs: A dict of
             {from_object_id: {to_object_id:
-                {'creator': `IPerson`, 'metadata': value}}}.
-            The creator and metadata keys are optional.
+                {'creator': `IPerson`, 'date_created': `datetime`,
+                 'metadata': value}}}.
+            The creator, date_created, and metadata keys are optional.
         """
 
     def findFromMany(object_ids, types=None):
@@ -39,8 +40,9 @@
         :param types: An optional collection of the types to include.
         :return: A dict of
             {from_object_id: {to_object_id:
-                {'creator': `IPerson`, 'metadata': value}}}.
-            The creator and metadata keys are optional.
+                {'creator': `IPerson`, 'date_created': `datetime`,
+                 'metadata': value}}}.
+            The creator, date_created, and metadata keys are optional.
         """
 
     def delete(xrefs):
@@ -57,6 +59,8 @@
         :param object_id: An object ID.
         :param types: An optional collection of the types to include.
         :return: A dict of
-            {to_object_id: {'creator': `IPerson`, 'metadata': value}}.
-            The creator and metadata keys are optional.
+            {to_object_id:
+                {'creator': `IPerson`, 'date_created': `datetime`,
+                 'metadata': value}}.
+            The creator, date_created, and metadata keys are optional.
         """


Follow ups