Launchpad itself

Merge ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master

  1. Git
  2. lp:~pappacena/launchpad
  3. git-repo-async-privacy-virtualrefs
  4. Merge into master
Proposed by Thiago F. Pappacena
Status: Needs review
Proposed branch: ~pappacena/launchpad:git-repo-async-privacy-virtualrefs
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:git-repo-async-provacy-garbo
Diff against target: 1053 lines (+547/-36)
14 files modified
database/schema/security.cfg (+1/-1)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+1/-1)
lib/lp/code/interfaces/branchmergeproposal.py (+45/-1)
lib/lp/code/interfaces/githosting.py (+8/-0)
lib/lp/code/interfaces/gitrepository.py (+12/-1)
lib/lp/code/model/branchmergeproposal.py (+98/-0)
lib/lp/code/model/githosting.py (+29/-4)
lib/lp/code/model/gitjob.py (+26/-1)
lib/lp/code/model/gitrepository.py (+5/-2)
lib/lp/code/model/tests/test_branchmergeproposal.py (+234/-0)
lib/lp/code/model/tests/test_githosting.py (+2/-3)
lib/lp/code/model/tests/test_gitrepository.py (+75/-20)
lib/lp/code/subscribers/branchmergeproposal.py (+8/-2)
lib/lp/code/tests/helpers.py (+3/-0)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+393075@code.launchpad.net

This proposal supersedes a proposal from 2020-10-29.

Commit message

Doing virtual refs synchronization when running git repository privacy change background task

Description of the change

This branch carries code from https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/390581, but it does a big chunch of refactoring of the code contained there. Mostly, the bulk operations got encapsulated in the GitHostingClient, so we would have a single point of change once Turnip will be able to run the copy/delete refs operations in bulk (instead of doing a bunch of requests).

To post a comment you must log in.

Unmerged commits

0f268fd... by Thiago F. Pappacena

Small bug fixes on permission change job

e954159... by Thiago F. Pappacena

Merge branch 'git-repo-async-privacy' into git-repo-async-privacy-virtualrefs

974b22a... by Thiago F. Pappacena

Fixing test

6fde200... by Thiago F. Pappacena

Refactoring the way we deal with copy/delete virtual refs operations

7793c43... by Thiago F. Pappacena

Merge branch 'create-mp-refs' into git-repo-async-privacy-virtualrefs

b4af659... by Thiago F. Pappacena

Merge branch 'master' into create-mp-refs

1688ced... by Thiago F. Pappacena

Running the branch sync when changing repository privacy

6dbcc2a... by Thiago F. Pappacena

Merge branch 'create-mp-refs' into git-repo-async-privacy-virtualrefs

6118345... by Thiago F. Pappacena

Improving tests for more scenarios

20ea95f... by Thiago F. Pappacena

Adding garbo to keep GitRepo status in line with info type change job

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/database/schema/security.cfg b/database/schema/security.cfg
2index 290e7bc..94cb548 100644
3--- a/database/schema/security.cfg
4+++ b/database/schema/security.cfg
5@@ -2083,7 +2083,7 @@ public.xref = SELECT, INSERT, DELETE
6 type=user
7
8 [privacy-change-jobs]
9-groups=script
10+groups=script, branchscanner
11 public.accessartifact = SELECT, UPDATE, DELETE, INSERT
12 public.accessartifactgrant = SELECT, UPDATE, DELETE, INSERT
13 public.accesspolicyartifact = SELECT, UPDATE, DELETE, INSERT
14diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
15index f2ec4ec..83c51dc 100644
16--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
17+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
18@@ -2336,7 +2336,7 @@ class TestLatestProposalsForEachBranchGit(
19
20 @staticmethod
21 def _setBranchInvisible(branch):
22- removeSecurityProxy(branch.repository).transitionToInformationType(
23+ removeSecurityProxy(branch.repository)._transitionToInformationType(
24 InformationType.USERDATA, branch.owner, verify_policy=False)
25
26
27diff --git a/lib/lp/code/interfaces/branchmergeproposal.py b/lib/lp/code/interfaces/branchmergeproposal.py
28index bf481c9..efe1e4a 100644
29--- a/lib/lp/code/interfaces/branchmergeproposal.py
30+++ b/lib/lp/code/interfaces/branchmergeproposal.py
31@@ -1,4 +1,4 @@
32-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
33+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
34 # GNU Affero General Public License version 3 (see the file LICENSE).
35
36 """The interface for branch merge proposals."""
37@@ -519,6 +519,50 @@ class IBranchMergeProposalView(Interface):
38 def getLatestDiffUpdateJob():
39 """Return the latest IUpdatePreviewDiffJob for this MP."""
40
41+ def getGitRefCopyOperations():
42+ """Gets the list of GitHosting copy operations that should be done
43+ in order to keep this MP's virtual refs up-to-date.
44+
45+ Note that, for now, the only possible virtual ref that could
46+ possibly be copied is GIT_CREATE_MP_VIRTUAL_REF. So, this method
47+ will return at most 1 copy operation. Once new virtual refs will be
48+ created, this method should be extended to add more copy operations
49+ too.
50+
51+ :return: A list of RefCopyOperation objects.
52+ """
53+
54+ def getGitRefDeleteOperations(force_delete=False):
55+ """Gets the list of git refs that should be deleted in order to keep
56+ this MP's virtual refs up-to-date.
57+
58+ Note that, for now, the only existing virtual ref to be deleted is
59+ GIT_CREATE_MP_VIRTUAL_REF. So this method will only ever delete at
60+ most 1 virtual ref. Once new virtual refs will be created, this method
61+ should be extended to delete them also.
62+
63+ :param force_delete: True if we should get delete operation
64+ regardless of any check. False otherwise.
65+ :return: A list of ref names.
66+ """
67+
68+ def syncGitVirtualRefs(force_delete=False):
69+ """Requests all copies and deletion of virtual refs to make git code
70+ hosting in sync with this MP.
71+
72+ :param force_delete: Do not try to copy any new virtual ref. Just
73+ delete all virtual refs possibly created.
74+ """
75+
76+ def bulkSyncGitVirtualRefs(merge_proposals):
77+ """Synchronizes a set of merge proposals' virtual refs.
78+
79+ :params merge_proposals: The set of merge proposals that should have
80+ the virtual refs synced.
81+ :return: A tuple with the list of (copy operations, delete operations)
82+ executed.
83+ """
84+
85
86 class IBranchMergeProposalEdit(Interface):
87
88diff --git a/lib/lp/code/interfaces/githosting.py b/lib/lp/code/interfaces/githosting.py
89index 441e850..7fe0d0e 100644
90--- a/lib/lp/code/interfaces/githosting.py
91+++ b/lib/lp/code/interfaces/githosting.py
92@@ -148,3 +148,11 @@ class IGitHostingClient(Interface):
93 :param refs: A list of tuples like (repo_path, ref_name) to be deleted.
94 :param logger: An optional logger.
95 """
96+
97+ def bulkSyncRefs(copy_operations, delete_operations):
98+ """Executes all copy operations and delete operations on Turnip
99+ side.
100+
101+ :param copy_operations: The list of RefCopyOperation to run.
102+ :param delete_operations: The list of RefDeleteOperation to run.
103+ """
104diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
105index 192133a..48e7dd3 100644
106--- a/lib/lp/code/interfaces/gitrepository.py
107+++ b/lib/lp/code/interfaces/gitrepository.py
108@@ -8,6 +8,8 @@ __metaclass__ = type
109 __all__ = [
110 'ContributorGitIdentity',
111 'GitIdentityMixin',
112+ 'GIT_CREATE_MP_VIRTUAL_REF',
113+ 'GIT_MP_VIRTUAL_REF_FORMAT',
114 'GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE',
115 'git_repository_name_validator',
116 'IGitRepository',
117@@ -105,6 +107,11 @@ valid_git_repository_name_pattern = re.compile(
118 r"^(?i)[a-z0-9][a-z0-9+\.\-@_]*\Z")
119
120
121+# Virtual ref where we automatically put a copy of any open merge proposal.
122+GIT_MP_VIRTUAL_REF_FORMAT = 'refs/merge/{mp.id}/head'
123+GIT_CREATE_MP_VIRTUAL_REF = 'git.mergeproposal_virtualref.enabled'
124+
125+
126 def valid_git_repository_name(name):
127 """Return True iff the name is valid as a Git repository name.
128
129@@ -595,11 +602,15 @@ class IGitRepositoryView(IHasRecipes):
130 def updateLandingTargets(paths):
131 """Update landing targets (MPs where this repository is the source).
132
133- For each merge proposal, create `UpdatePreviewDiffJob`s.
134+ For each merge proposal, create `UpdatePreviewDiffJob`s, and runs
135+ the appropriate GitHosting.copyRefs operation to allow having
136+ virtual merge refs with the updated branch version.
137
138 :param paths: A list of reference paths. Any merge proposals whose
139 source is this repository and one of these paths will have their
140 diffs updated.
141+ :return: Returns a tuple with the list of background jobs created,
142+ and the list of ref copies requested to GitHosting.
143 """
144
145 def markRecipesStale(paths):
146diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
147index 061fb1e..821b15d 100644
148--- a/lib/lp/code/model/branchmergeproposal.py
149+++ b/lib/lp/code/model/branchmergeproposal.py
150@@ -12,6 +12,7 @@ __all__ = [
151
152 from collections import defaultdict
153 from email.utils import make_msgid
154+import logging
155 from operator import attrgetter
156 import sys
157
158@@ -84,7 +85,12 @@ from lp.code.interfaces.codereviewcomment import ICodeReviewComment
159 from lp.code.interfaces.codereviewinlinecomment import (
160 ICodeReviewInlineCommentSet,
161 )
162+from lp.code.interfaces.githosting import IGitHostingClient
163 from lp.code.interfaces.gitref import IGitRef
164+from lp.code.interfaces.gitrepository import (
165+ GIT_CREATE_MP_VIRTUAL_REF,
166+ GIT_MP_VIRTUAL_REF_FORMAT,
167+ )
168 from lp.code.mail.branch import RecipientReason
169 from lp.code.model.branchrevision import BranchRevision
170 from lp.code.model.codereviewcomment import CodeReviewComment
171@@ -94,6 +100,10 @@ from lp.code.model.diff import (
172 IncrementalDiff,
173 PreviewDiff,
174 )
175+from lp.code.model.githosting import (
176+ RefCopyOperation,
177+ RefDeleteOperation,
178+ )
179 from lp.registry.interfaces.person import (
180 IPerson,
181 IPersonSet,
182@@ -123,6 +133,7 @@ from lp.services.database.sqlbase import (
183 quote,
184 SQLBase,
185 )
186+from lp.services.features import getFeatureFlag
187 from lp.services.helpers import shortlist
188 from lp.services.job.interfaces.job import JobStatus
189 from lp.services.job.model.job import Job
190@@ -144,6 +155,9 @@ from lp.soyuz.enums import (
191 )
192
193
194+logger = logging.getLogger(__name__)
195+
196+
197 def is_valid_transition(proposal, from_state, next_state, user=None):
198 """Is it valid for this user to move this proposal to to next_state?
199
200@@ -971,6 +985,7 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
201 branch_merge_proposal=self.id):
202 job.destroySelf()
203 self._preview_diffs.remove()
204+
205 self.destroySelf()
206
207 def getUnlandedSourceBranchRevisions(self):
208@@ -1217,6 +1232,89 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
209 for diff in diffs)
210 return [diff_dict.get(revisions) for revisions in revision_list]
211
212+ @property
213+ def should_have_git_virtual_refs(self):
214+ """True if this MP should have virtual refs in the target
215+ repository. False otherwise."""
216+ # If the source repository is private and it's opening a MP to
217+ # another repository, we should not risk disclosing the private code to
218+ # everyone with permission to see the target repo:
219+ private_source = self.source_git_repository.private
220+ same_owner = self.source_git_repository.owner.inTeam(
221+ self.target_git_repository.owner)
222+ return not private_source or same_owner
223+
224+ def getGitRefCopyOperations(self):
225+ """See `IBranchMergeProposal`"""
226+ if (not self.target_git_repository
227+ or not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF)):
228+ # Not a git MP. Skip.
229+ return []
230+ if not self.should_have_git_virtual_refs:
231+ return []
232+ return [RefCopyOperation(
233+ self.source_git_repository.getInternalPath(),
234+ self.source_git_commit_sha1,
235+ self.target_git_repository.getInternalPath(),
236+ GIT_MP_VIRTUAL_REF_FORMAT.format(mp=self))]
237+
238+ def getGitRefDeleteOperations(self, force_delete=False):
239+ """See `IBranchMergeProposal`"""
240+ if self.source_git_ref is None:
241+ # Not a git MP. Skip.
242+ return []
243+ if not force_delete and self.should_have_git_virtual_refs:
244+ return []
245+ return [RefDeleteOperation(
246+ self.target_git_repository.getInternalPath(),
247+ GIT_MP_VIRTUAL_REF_FORMAT.format(mp=self))]
248+
249+ def syncGitVirtualRefs(self, force_delete=False):
250+ """See `IBranchMergeProposal`"""
251+ if self.source_git_ref is None:
252+ return
253+ if not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF):
254+ # XXX pappacena 2020-09-15: Do not try to remove virtual refs if
255+ # the feature is disabled. It might be disabled due to bug on
256+ # the first versions, for example, and we should have a way to
257+ # skip this feature entirely.
258+ # Once the feature is stable, we should actually *always* delete
259+ # the virtual refs, since we don't know if the feature was
260+ # enabled or not when the branch was created.
261+ return
262+ if force_delete:
263+ copy_operations = []
264+ else:
265+ copy_operations = self.getGitRefCopyOperations()
266+ delete_operations = self.getGitRefDeleteOperations(force_delete)
267+
268+ if copy_operations or delete_operations:
269+ hosting_client = getUtility(IGitHostingClient)
270+ hosting_client.bulkSyncRefs(copy_operations, delete_operations)
271+
272+ @classmethod
273+ def bulkSyncGitVirtualRefs(cls, merge_proposals):
274+ """See `IBranchMergeProposal`"""
275+ if not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF):
276+ # XXX pappacena 2020-09-15: Do not try to remove virtual refs if
277+ # the feature is disabled. It might be disabled due to bug on
278+ # the first versions, for example, and we should have a way to
279+ # skip this feature entirely.
280+ # Once the feature is stable, we should actually *always* delete
281+ # the virtual refs, since we don't know if the feature was
282+ # enabled or not when the branch was created.
283+ return [], []
284+ copy_operations = []
285+ delete_operations = []
286+ for merge_proposal in merge_proposals:
287+ copy_operations += merge_proposal.getGitRefCopyOperations()
288+ delete_operations += merge_proposal.getGitRefDeleteOperations()
289+
290+ if copy_operations or delete_operations:
291+ hosting_client = getUtility(IGitHostingClient)
292+ hosting_client.bulkSyncRefs(copy_operations, delete_operations)
293+ return copy_operations, delete_operations
294+
295 def scheduleDiffUpdates(self, return_jobs=True):
296 """See `IBranchMergeProposal`."""
297 from lp.code.model.branchmergeproposaljob import (
298diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py
299index 74f4ec0..d0e23cf 100644
300--- a/lib/lp/code/model/githosting.py
301+++ b/lib/lp/code/model/githosting.py
302@@ -7,9 +7,11 @@ __metaclass__ = type
303 __all__ = [
304 'GitHostingClient',
305 'RefCopyOperation',
306+ 'RefDeleteOperation',
307 ]
308
309 import base64
310+from collections import defaultdict
311 import json
312 import sys
313
314@@ -33,7 +35,6 @@ from lp.code.errors import (
315 GitRepositoryDeletionFault,
316 GitRepositoryScanFault,
317 GitTargetError,
318- NoSuchGitReference,
319 )
320 from lp.code.interfaces.githosting import IGitHostingClient
321 from lp.services.config import config
322@@ -55,12 +56,21 @@ class RefCopyOperation:
323 This class is just a helper to define copy operations parameters on
324 IGitHostingClient.copyRefs method.
325 """
326- def __init__(self, source_ref, target_repo, target_ref):
327+ def __init__(self, source_repo_path, source_ref, target_repo_path,
328+ target_ref):
329+ self.source_repo_path = source_repo_path
330 self.source_ref = source_ref
331- self.target_repo = target_repo
332+ self.target_repo_path = target_repo_path
333 self.target_ref = target_ref
334
335
336+class RefDeleteOperation:
337+ """A description of a ref delete operation."""
338+ def __init__(self, repo_path, ref):
339+ self.repo_path = repo_path
340+ self.ref = ref
341+
342+
343 @implementer(IGitHostingClient)
344 class GitHostingClient:
345 """A client for the internal API provided by the Git hosting system."""
346@@ -277,7 +287,7 @@ class GitHostingClient:
347 json_data = {
348 "operations": [{
349 "from": i.source_ref,
350- "to": {"repo": i.target_repo, "ref": i.target_ref}
351+ "to": {"repo": i.target_repo_path, "ref": i.target_ref}
352 } for i in operations]
353 }
354 try:
355@@ -299,6 +309,8 @@ class GitHostingClient:
356
357 def deleteRefs(self, refs, logger=None):
358 """See `IGitHostingClient`."""
359+ # XXX pappacena: This needs to be done in a bulk operation,
360+ # instead of several requests.
361 for path, ref in refs:
362 try:
363 if logger is not None:
364@@ -309,3 +321,16 @@ class GitHostingClient:
365 raise GitReferenceDeletionFault(
366 "Error deleting %s from repo %s: HTTP %s" %
367 (ref, path, e.response.status_code))
368+
369+ def bulkSyncRefs(self, copy_operations, delete_operations):
370+ """See `IGitHostingClient`."""
371+ # XXX pappacena: This needs to be done in a bulk operation on Turnip
372+ # side, instead of several requests.
373+ operations_per_path = defaultdict(list)
374+ for operation in copy_operations:
375+ operations_per_path[operation.source_repo_path].append(operation)
376+ for repo_path, operations in operations_per_path.items():
377+ self.copyRefs(repo_path, operations)
378+
379+ self.deleteRefs([(i.repo_path, i.ref) for i in delete_operations])
380+
381diff --git a/lib/lp/code/model/gitjob.py b/lib/lp/code/model/gitjob.py
382index ba7b9f2..4514f08 100644
383--- a/lib/lp/code/model/gitjob.py
384+++ b/lib/lp/code/model/gitjob.py
385@@ -12,6 +12,7 @@ __all__ = [
386 'ReclaimGitRepositorySpaceJob',
387 ]
388
389+from itertools import chain
390
391 from lazr.delegates import delegate_to
392 from lazr.enum import (
393@@ -33,7 +34,10 @@ from zope.interface import (
394 provider,
395 )
396
397-from lp.app.enums import InformationType
398+from lp.app.enums import (
399+ InformationType,
400+ PRIVATE_INFORMATION_TYPES,
401+ )
402 from lp.app.errors import NotFoundError
403 from lp.code.enums import (
404 GitActivityType,
405@@ -54,6 +58,7 @@ from lp.code.interfaces.gitjob import (
406 )
407 from lp.code.interfaces.gitrule import describe_git_permissions
408 from lp.code.mail.branch import BranchMailer
409+from lp.code.model.branchmergeproposal import BranchMergeProposal
410 from lp.registry.interfaces.person import IPersonSet
411 from lp.services.config import config
412 from lp.services.database.enumcol import EnumCol
413@@ -440,6 +445,12 @@ class GitRepositoryTransitionToInformationTypeJob(GitJobDerived):
414 def information_type(self):
415 return InformationType.items[self.metadata["information_type"]]
416
417+ def updateVirtualRefs(self):
418+ merge_proposals = chain(
419+ self.repository.landing_targets,
420+ self.repository.landing_candidates)
421+ BranchMergeProposal.bulkSyncGitVirtualRefs(merge_proposals)
422+
423 def run(self):
424 """See `IGitRepositoryTransitionToInformationTypeJob`."""
425 if (self.repository.status !=
426@@ -447,7 +458,21 @@ class GitRepositoryTransitionToInformationTypeJob(GitJobDerived):
427 raise AttributeError(
428 "The repository %s is not pending information type change." %
429 self.repository)
430+ is_private = self.repository.private
431+ will_be_private = self.information_type in PRIVATE_INFORMATION_TYPES
432
433+ # XXX pappacena 2020-10-29: We might need to run the
434+ # _transitionToInformationType as a callback (maybe on
435+ # lp.code.xmlrpc.git), as a reaction to git hosting server finishing
436+ # the operations to update virtual refs (call bellow).
437 self.repository._transitionToInformationType(
438 self.information_type, self.user, self.verify_policy)
439+
440+ # When changing privacy type, we need to make sure to not leak
441+ # anything though virtual refs, and create new ones that didn't
442+ # exist before.
443+ if is_private != will_be_private:
444+ # XXX pappacena 2020-10-28: We need to implement retry rules here.
445+ self.updateVirtualRefs()
446+
447 self.repository.status = GitRepositoryStatus.AVAILABLE
448diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
449index fb506f1..731ca3f 100644
450--- a/lib/lp/code/model/gitrepository.py
451+++ b/lib/lp/code/model/gitrepository.py
452@@ -1205,9 +1205,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
453 def updateLandingTargets(self, paths):
454 """See `IGitRepository`."""
455 jobs = []
456- for merge_proposal in self.getActiveLandingTargets(paths):
457+ merge_proposals = self.getActiveLandingTargets(paths)
458+ for merge_proposal in merge_proposals:
459 jobs.extend(merge_proposal.scheduleDiffUpdates())
460- return jobs
461+ copy_ops, delete_ops = BranchMergeProposal.bulkSyncGitVirtualRefs(
462+ merge_proposals)
463+ return jobs, copy_ops, delete_ops
464
465 def _getRecipes(self, paths=None):
466 """Undecorated version of recipes for use by `markRecipesStale`."""
467diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
468index 81b94b3..db69320 100644
469--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
470+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
471@@ -67,6 +67,10 @@ from lp.code.interfaces.branchmergeproposal import (
472 IBranchMergeProposalGetter,
473 IBranchMergeProposalJobSource,
474 )
475+from lp.code.interfaces.gitrepository import (
476+ GIT_CREATE_MP_VIRTUAL_REF,
477+ GIT_MP_VIRTUAL_REF_FORMAT,
478+ )
479 from lp.code.model.branchmergeproposal import (
480 BranchMergeProposal,
481 BranchMergeProposalGetter,
482@@ -97,6 +101,7 @@ from lp.testing import (
483 ExpectedException,
484 launchpadlib_for,
485 login,
486+ login_admin,
487 login_person,
488 person_logged_in,
489 TestCaseWithFactory,
490@@ -256,6 +261,196 @@ class TestBranchMergeProposalPrivacy(TestCaseWithFactory):
491 self.assertContentEqual([owner, team], subscriptions)
492
493
494+class TestGitBranchMergeProposalVirtualRefs(TestCaseWithFactory):
495+ """Ensure that BranchMergeProposal creation run the appropriate copy
496+ and delete of virtual refs, like ref/merge/<id>/head."""
497+
498+ layer = DatabaseFunctionalLayer
499+
500+ def setUp(self):
501+ super(TestGitBranchMergeProposalVirtualRefs, self).setUp()
502+ self.hosting_fixture = self.useFixture(GitHostingFixture())
503+
504+ def test_should_have_vrefs_public_repos(self):
505+ mp = self.factory.makeBranchMergeProposalForGit()
506+ self.assertTrue(mp.should_have_git_virtual_refs)
507+
508+ def test_should_have_vrefs_private_source(self):
509+ login_admin()
510+ source_repo = self.factory.makeGitRepository(
511+ information_type=InformationType.PRIVATESECURITY)
512+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
513+ [source] = self.factory.makeGitRefs(source_repo)
514+ [target] = self.factory.makeGitRefs(target_repo)
515+ mp = self.factory.makeBranchMergeProposalForGit(
516+ source_ref=source, target_ref=target)
517+ self.assertFalse(mp.should_have_git_virtual_refs)
518+
519+ def test_should_have_vrefs_private_target(self):
520+ login_admin()
521+ source_repo = self.factory.makeGitRepository()
522+ target_repo = self.factory.makeGitRepository(
523+ target=source_repo.target,
524+ information_type=InformationType.PRIVATESECURITY)
525+ [source] = self.factory.makeGitRefs(source_repo)
526+ [target] = self.factory.makeGitRefs(target_repo)
527+ mp = self.factory.makeBranchMergeProposalForGit(
528+ source_ref=source, target_ref=target)
529+ self.assertTrue(mp.should_have_git_virtual_refs)
530+
531+ def test_copy_git_merge_virtual_ref(self):
532+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
533+ mp = self.factory.makeBranchMergeProposalForGit()
534+
535+ copy_operations = mp.getGitRefCopyOperations()
536+ self.assertEqual(1, len(copy_operations))
537+ self.assertThat(copy_operations[0], MatchesStructure(
538+ source_repo_path=Equals(
539+ mp.source_git_repository.getInternalPath()),
540+ source_ref=Equals(mp.source_git_commit_sha1),
541+ target_repo_path=Equals(
542+ mp.target_git_repository.getInternalPath()),
543+ target_ref=Equals("refs/merge/%s/head" % mp.id),
544+ ))
545+ delete_operations = mp.getGitRefDeleteOperations()
546+ self.assertEqual([], delete_operations)
547+
548+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
549+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
550+ copy_ops, delete_ops = args
551+ self.assertEqual({}, kwargs)
552+ self.assertEqual([], delete_ops)
553+ self.assertEqual(1, len(copy_ops))
554+ self.assertThat(copy_ops[0], MatchesStructure(
555+ source_repo_path=Equals(
556+ mp.source_git_repository.getInternalPath()),
557+ source_ref=Equals(mp.source_git_commit_sha1),
558+ target_repo_path=Equals(
559+ mp.target_git_repository.getInternalPath()),
560+ target_ref=Equals("refs/merge/%s/head" % mp.id),
561+ ))
562+
563+ def test_getGitRefCopyOperations_private_source(self):
564+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
565+ login_admin()
566+ source_repo = self.factory.makeGitRepository(
567+ information_type=InformationType.PRIVATESECURITY)
568+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
569+ [source] = self.factory.makeGitRefs(source_repo)
570+ [target] = self.factory.makeGitRefs(target_repo)
571+ mp = self.factory.makeBranchMergeProposalForGit(
572+ source_ref=source, target_ref=target)
573+ self.assertEqual([], mp.getGitRefCopyOperations())
574+
575+ delete_operations = mp.getGitRefDeleteOperations()
576+ self.assertEqual(1, len(delete_operations))
577+ self.assertThat(delete_operations[0], MatchesStructure(
578+ repo_path=Equals(mp.target_git_repository.getInternalPath()),
579+ ref=Equals("refs/merge/%s/head" % mp.id),
580+ ))
581+
582+ def test_getGitRefCopyOperations_private_source_same_repo(self):
583+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
584+ login_admin()
585+ repo = self.factory.makeGitRepository(
586+ information_type=InformationType.PRIVATESECURITY)
587+ [source, target] = self.factory.makeGitRefs(
588+ repo, ['refs/heads/bugfix', 'refs/heads/master'])
589+ mp = self.factory.makeBranchMergeProposalForGit(
590+ source_ref=source, target_ref=target)
591+ operations = mp.getGitRefCopyOperations()
592+ self.assertEqual(1, len(operations))
593+ self.assertThat(operations[0], MatchesStructure(
594+ source_repo_path=Equals(
595+ mp.source_git_repository.getInternalPath()),
596+ source_ref=Equals(mp.source_git_commit_sha1),
597+ target_repo_path=Equals(
598+ mp.target_git_repository.getInternalPath()),
599+ target_ref=Equals(GIT_MP_VIRTUAL_REF_FORMAT.format(mp=mp))
600+ ))
601+
602+ delete_operations = mp.getGitRefDeleteOperations()
603+ self.assertEqual([], delete_operations)
604+
605+ def test_getGitRefCopyOperations_private_source_same_owner(self):
606+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
607+ login_admin()
608+ source_repo = self.factory.makeGitRepository(
609+ information_type=InformationType.PRIVATESECURITY)
610+ target_repo = self.factory.makeGitRepository(
611+ target=source_repo.target, owner=source_repo.owner)
612+ [source] = self.factory.makeGitRefs(source_repo)
613+ [target] = self.factory.makeGitRefs(target_repo)
614+ mp = self.factory.makeBranchMergeProposalForGit(
615+ source_ref=source, target_ref=target)
616+ operations = mp.getGitRefCopyOperations()
617+ self.assertEqual(1, len(operations))
618+ self.assertThat(operations[0], MatchesStructure(
619+ source_repo_path=Equals(
620+ mp.source_git_repository.getInternalPath()),
621+ source_ref=Equals(mp.source_git_commit_sha1),
622+ target_repo_path=Equals(
623+ mp.target_git_repository.getInternalPath()),
624+ target_ref=Equals(GIT_MP_VIRTUAL_REF_FORMAT.format(mp=mp))
625+ ))
626+
627+ delete_operations = mp.getGitRefDeleteOperations()
628+ self.assertEqual([], delete_operations)
629+
630+ def test_syncGitVirtualRefs(self):
631+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
632+ login_admin()
633+ login_admin()
634+ source_repo = self.factory.makeGitRepository()
635+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
636+ [source] = self.factory.makeGitRefs(source_repo)
637+ [target] = self.factory.makeGitRefs(target_repo)
638+ mp = self.factory.makeBranchMergeProposalForGit(
639+ source_ref=source, target_ref=target)
640+
641+ # mp.syncGitVirtualRefs should have been triggered by event.
642+ # Check lp.code.subscribers.branchmergeproposal.merge_proposal_created.
643+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
644+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
645+ copy_ops, delete_ops = args
646+ self.assertEquals({}, kwargs)
647+ self.assertEqual([], delete_ops)
648+ self.assertEqual(1, len(copy_ops))
649+ self.assertThat(copy_ops[0], MatchesStructure(
650+ source_repo_path=Equals(
651+ mp.source_git_repository.getInternalPath()),
652+ source_ref=Equals(mp.source_git_commit_sha1),
653+ target_repo_path=Equals(
654+ mp.target_git_repository.getInternalPath()),
655+ target_ref=Equals("refs/merge/%s/head" % mp.id),
656+ ))
657+
658+ self.assertEqual(0, self.hosting_fixture.deleteRefs.call_count)
659+
660+ def test_syncGitVirtualRefs_private_source_deletes_ref(self):
661+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
662+ login_admin()
663+ source_repo = self.factory.makeGitRepository(
664+ information_type=InformationType.PRIVATESECURITY)
665+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
666+ [source] = self.factory.makeGitRefs(source_repo)
667+ [target] = self.factory.makeGitRefs(target_repo)
668+ mp = self.factory.makeBranchMergeProposalForGit(
669+ source_ref=source, target_ref=target)
670+
671+ # mp.syncGitVirtualRefs should have been triggered by event.
672+ # Check lp.code.subscribers.branchmergeproposal.merge_proposal_deleted.
673+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
674+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
675+ copy_ops, delete_ops = args
676+ self.assertEqual({}, kwargs)
677+ self.assertEqual([], copy_ops)
678+ self.assertEqual(1, len(delete_ops))
679+ self.assertThat(delete_ops[0], MatchesStructure(
680+ repo_path=Equals(target_repo.getInternalPath()),
681+ ref=Equals("refs/merge/%s/head" % mp.id)))
682+
683+
684 class TestBranchMergeProposalTransitions(TestCaseWithFactory):
685 """Test the state transitions of branch merge proposals."""
686
687@@ -1185,6 +1380,10 @@ class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
688 def test_delete_triggers_webhooks(self):
689 # When an existing merge proposal is deleted, any relevant webhooks
690 # are triggered.
691+ self.useFixture(FeatureFixture({
692+ GIT_CREATE_MP_VIRTUAL_REF: "on",
693+ BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG: "on"}))
694+ hosting_fixture = self.useFixture(GitHostingFixture())
695 logger = self.useFixture(FakeLogger())
696 source = self.makeBranch()
697 target = self.makeBranch(same_target_as=source)
698@@ -1203,10 +1402,25 @@ class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
699 expected_redacted_payload = dict(
700 expected_payload,
701 old=MatchesDict(self.getExpectedPayload(proposal, redact=True)))
702+ hosting_fixture = self.useFixture(GitHostingFixture())
703 proposal.deleteProposal()
704 delivery = hook.deliveries.one()
705+ self.assertIsNotNone(delivery)
706 self.assertCorrectDelivery(expected_payload, hook, delivery)
707 self.assertCorrectLogging(expected_redacted_payload, hook, logger)
708+ if not self.git:
709+ self.assertEqual(0, hosting_fixture.bulkSyncRefs.call_count)
710+ else:
711+ self.assertEqual(1, hosting_fixture.bulkSyncRefs.call_count)
712+ args, kwargs = hosting_fixture.bulkSyncRefs.calls[0]
713+ self.assertEqual({}, kwargs)
714+ copy_ops, delete_ops = args
715+ self.assertEqual(0, len(copy_ops))
716+ self.assertEqual(1, len(delete_ops))
717+ self.assertThat(delete_ops[0], MatchesStructure(
718+ repo_path=Equals(target.repository.getInternalPath()),
719+ ref=Equals("refs/merge/%s/head" % proposal.id)
720+ ))
721
722
723 class TestGetAddress(TestCaseWithFactory):
724@@ -1507,6 +1721,26 @@ class TestBranchMergeProposalDeletion(TestCaseWithFactory):
725 self.assertRaises(
726 SQLObjectNotFound, BranchMergeProposalJob.get, job_id)
727
728+ def test_deleteProposal_for_git_removes_virtual_ref(self):
729+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
730+ self.useFixture(GitHostingFixture())
731+ proposal = self.factory.makeBranchMergeProposalForGit()
732+
733+ # Clean up fixture calls.
734+ hosting_fixture = self.useFixture(GitHostingFixture())
735+ proposal.deleteProposal()
736+
737+ self.assertEqual(1, hosting_fixture.bulkSyncRefs.call_count)
738+ args, kwargs = hosting_fixture.bulkSyncRefs.calls[0]
739+ self.assertEqual({}, kwargs)
740+ copy_ops, delete_ops = args
741+ self.assertEqual([], copy_ops)
742+ self.assertEqual(1, len(delete_ops))
743+ self.assertThat(delete_ops[0], MatchesStructure(
744+ repo_path=Equals(proposal.target_git_repository.getInternalPath()),
745+ ref=Equals('refs/merge/%s/head' % proposal.id)
746+ ))
747+
748
749 class TestBranchMergeProposalBugs(WithVCSScenarios, TestCaseWithFactory):
750
751diff --git a/lib/lp/code/model/tests/test_githosting.py b/lib/lp/code/model/tests/test_githosting.py
752index 23d6063..fad0df3 100644
753--- a/lib/lp/code/model/tests/test_githosting.py
754+++ b/lib/lp/code/model/tests/test_githosting.py
755@@ -42,7 +42,6 @@ from lp.code.errors import (
756 GitRepositoryDeletionFault,
757 GitRepositoryScanFault,
758 GitTargetError,
759- NoSuchGitReference,
760 )
761 from lp.code.interfaces.githosting import IGitHostingClient
762 from lp.code.model.githosting import RefCopyOperation
763@@ -414,8 +413,8 @@ class TestGitHostingClient(TestCase):
764
765 def getCopyRefOperations(self):
766 return [
767- RefCopyOperation("1a2b3c4", "999", "refs/merge/123"),
768- RefCopyOperation("9a8b7c6", "666", "refs/merge/989"),
769+ RefCopyOperation("1", "1a2b3c4", "999", "refs/merge/123"),
770+ RefCopyOperation("1", "9a8b7c6", "666", "refs/merge/989"),
771 ]
772
773 def test_copyRefs(self):
774diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
775index 08887f5..2296352 100644
776--- a/lib/lp/code/model/tests/test_gitrepository.py
777+++ b/lib/lp/code/model/tests/test_gitrepository.py
778@@ -97,6 +97,7 @@ from lp.code.interfaces.gitnamespace import (
779 IGitNamespaceSet,
780 )
781 from lp.code.interfaces.gitrepository import (
782+ GIT_CREATE_MP_VIRTUAL_REF,
783 IGitRepository,
784 IGitRepositorySet,
785 IGitRepositoryView,
786@@ -783,6 +784,7 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
787 # Make sure that the tests all flush the database changes.
788 self.addCleanup(Store.of(self.repository).flush)
789 login_person(self.user)
790+ self.hosting_fixture = self.useFixture(GitHostingFixture())
791
792 def test_deletable(self):
793 # A newly created repository can be deleted without any problems.
794@@ -824,7 +826,6 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
795
796 def test_code_import_does_not_disable_deletion(self):
797 # A repository that has an attached code import can be deleted.
798- self.useFixture(GitHostingFixture())
799 code_import = self.factory.makeCodeImport(
800 target_rcs_type=TargetRevisionControlSystems.GIT)
801 repository = code_import.git_repository
802@@ -984,6 +985,7 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
803 # unsubscribe the repository owner here.
804 self.repository.unsubscribe(
805 self.repository.owner, self.repository.owner)
806+ self.hosting_fixture = self.useFixture(GitHostingFixture())
807
808 def test_plain_repository(self):
809 # A fresh repository has no deletion requirements.
810@@ -1115,7 +1117,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
811
812 def test_code_import_requirements(self):
813 # Code imports are not included explicitly in deletion requirements.
814- self.useFixture(GitHostingFixture())
815 code_import = self.factory.makeCodeImport(
816 target_rcs_type=TargetRevisionControlSystems.GIT)
817 # Remove the implicit repository subscription first.
818@@ -1126,7 +1127,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
819
820 def test_code_import_deletion(self):
821 # break_references allows deleting a code import repository.
822- self.useFixture(GitHostingFixture())
823 code_import = self.factory.makeCodeImport(
824 target_rcs_type=TargetRevisionControlSystems.GIT)
825 code_import_id = code_import.id
826@@ -1182,7 +1182,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
827
828 def test_DeleteCodeImport(self):
829 # DeleteCodeImport.__call__ must delete the CodeImport.
830- self.useFixture(GitHostingFixture())
831 code_import = self.factory.makeCodeImport(
832 target_rcs_type=TargetRevisionControlSystems.GIT)
833 code_import_id = code_import.id
834@@ -1521,6 +1520,10 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
835
836 layer = DatabaseFunctionalLayer
837
838+ def setUp(self):
839+ super(TestGitRepositoryRefs, self).setUp()
840+ self.hosting_fixture = self.useFixture(GitHostingFixture())
841+
842 def test__convertRefInfo(self):
843 # _convertRefInfo converts a valid info dictionary.
844 sha1 = six.ensure_text(hashlib.sha1("").hexdigest())
845@@ -1791,18 +1794,17 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
846 # planRefChanges excludes some ref prefixes by default, and can be
847 # configured otherwise.
848 repository = self.factory.makeGitRepository()
849- hosting_fixture = self.useFixture(GitHostingFixture())
850 repository.planRefChanges("dummy")
851 self.assertEqual(
852 [{"exclude_prefixes": ["refs/changes/"]}],
853- hosting_fixture.getRefs.extract_kwargs())
854- hosting_fixture.getRefs.calls = []
855+ self.hosting_fixture.getRefs.extract_kwargs())
856+ self.hosting_fixture.getRefs.calls = []
857 self.pushConfig(
858 "codehosting", git_exclude_ref_prefixes="refs/changes/ refs/pull/")
859 repository.planRefChanges("dummy")
860 self.assertEqual(
861 [{"exclude_prefixes": ["refs/changes/", "refs/pull/"]}],
862- hosting_fixture.getRefs.extract_kwargs())
863+ self.hosting_fixture.getRefs.extract_kwargs())
864
865 def test_fetchRefCommits(self):
866 # fetchRefCommits fetches detailed tip commit metadata for the
867@@ -1875,9 +1877,8 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
868 def test_fetchRefCommits_empty(self):
869 # If given an empty refs dictionary, fetchRefCommits returns early
870 # without contacting the hosting service.
871- hosting_fixture = self.useFixture(GitHostingFixture())
872 GitRepository.fetchRefCommits("dummy", {})
873- self.assertEqual([], hosting_fixture.getCommits.calls)
874+ self.assertEqual([], self.hosting_fixture.getCommits.calls)
875
876 def test_synchroniseRefs(self):
877 # synchroniseRefs copes with synchronising a repository where some
878@@ -1920,7 +1921,6 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
879 self.assertThat(repository.refs, MatchesSetwise(*matchers))
880
881 def test_set_default_branch(self):
882- hosting_fixture = self.useFixture(GitHostingFixture())
883 repository = self.factory.makeGitRepository()
884 self.factory.makeGitRefs(
885 repository=repository,
886@@ -1931,22 +1931,20 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
887 self.assertEqual(
888 [((repository.getInternalPath(),),
889 {"default_branch": "refs/heads/new"})],
890- hosting_fixture.setProperties.calls)
891+ self.hosting_fixture.setProperties.calls)
892 self.assertEqual("refs/heads/new", repository.default_branch)
893
894 def test_set_default_branch_unchanged(self):
895- hosting_fixture = self.useFixture(GitHostingFixture())
896 repository = self.factory.makeGitRepository()
897 self.factory.makeGitRefs(
898 repository=repository, paths=["refs/heads/master"])
899 removeSecurityProxy(repository)._default_branch = "refs/heads/master"
900 with person_logged_in(repository.owner):
901 repository.default_branch = "master"
902- self.assertEqual([], hosting_fixture.setProperties.calls)
903+ self.assertEqual([], self.hosting_fixture.setProperties.calls)
904 self.assertEqual("refs/heads/master", repository.default_branch)
905
906 def test_set_default_branch_imported(self):
907- hosting_fixture = self.useFixture(GitHostingFixture())
908 repository = self.factory.makeGitRepository(
909 repository_type=GitRepositoryType.IMPORTED)
910 self.factory.makeGitRefs(
911@@ -1959,7 +1957,7 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
912 "Cannot modify non-hosted Git repository %s." %
913 repository.display_name,
914 setattr, repository, "default_branch", "new")
915- self.assertEqual([], hosting_fixture.setProperties.calls)
916+ self.assertEqual([], self.hosting_fixture.setProperties.calls)
917 self.assertEqual("refs/heads/master", repository.default_branch)
918
919 def test_exception_unset_default_branch(self):
920@@ -2581,18 +2579,66 @@ class TestGitRepositoryUpdateLandingTargets(TestCaseWithFactory):
921
922 layer = DatabaseFunctionalLayer
923
924- def test_schedules_diff_updates(self):
925+ def setUp(self):
926+ super(TestGitRepositoryUpdateLandingTargets, self).setUp()
927+ self.hosting_fixture = self.useFixture(GitHostingFixture())
928+
929+ def assertSchedulesDiffUpdate(self, with_mp_virtual_ref):
930 """Create jobs for all merge proposals."""
931 bmp1 = self.factory.makeBranchMergeProposalForGit()
932 bmp2 = self.factory.makeBranchMergeProposalForGit(
933 source_ref=bmp1.source_git_ref)
934- jobs = bmp1.source_git_repository.updateLandingTargets(
935- [bmp1.source_git_path])
936+
937+ # Only enable this virtual ref here, since
938+ # self.factory.makeBranchMergeProposalForGit also tries to create
939+ # the virtual refs.
940+ if with_mp_virtual_ref:
941+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
942+ else:
943+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: ""}))
944+ jobs, ref_copies, ref_deletes = (
945+ bmp1.source_git_repository.updateLandingTargets(
946+ [bmp1.source_git_path]))
947 self.assertEqual(2, len(jobs))
948 bmps_to_update = [
949 removeSecurityProxy(job).branch_merge_proposal for job in jobs]
950 self.assertContentEqual([bmp1, bmp2], bmps_to_update)
951
952+ if not with_mp_virtual_ref:
953+ self.assertEqual(
954+ 0, self.hosting_fixture.bulkSyncRefs.call_count)
955+ else:
956+ self.assertEqual(
957+ 1, self.hosting_fixture.bulkSyncRefs.call_count)
958+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
959+ self.assertEqual({}, kwargs)
960+ self.assertEqual(2, len(args))
961+ copy_ops, delete_ops = args
962+ self.assertEqual(2, len(copy_ops))
963+ self.assertEqual(0, len(delete_ops))
964+ self.assertThat(copy_ops[0], MatchesStructure(
965+ source_repo_path=Equals(
966+ bmp1.source_git_repository.getInternalPath()),
967+ source_ref=Equals(bmp1.source_git_commit_sha1),
968+ target_repo_path=Equals(
969+ bmp1.target_git_repository.getInternalPath()),
970+ target_ref=Equals("refs/merge/%s/head" % bmp1.id),
971+ ))
972+ self.assertThat(copy_ops[1], MatchesStructure(
973+ source_repo_path=Equals(
974+ bmp2.source_git_repository.getInternalPath()),
975+ source_ref=Equals(bmp2.source_git_commit_sha1),
976+ target_repo_path=Equals(
977+ bmp2.target_git_repository.getInternalPath()),
978+ target_ref=Equals("refs/merge/%s/head" % bmp2.id),
979+ ))
980+
981+ def test_schedules_diff_updates_with_mp_virtual_ref(self):
982+ self.assertSchedulesDiffUpdate(with_mp_virtual_ref=True)
983+
984+ def test_schedules_diff_updates_without_mp_virtual_ref(self):
985+ self.assertSchedulesDiffUpdate(with_mp_virtual_ref=False)
986+
987 def test_ignores_final(self):
988 """Diffs for proposals in final states aren't updated."""
989 [source_ref] = self.factory.makeGitRefs()
990@@ -2604,8 +2650,17 @@ class TestGitRepositoryUpdateLandingTargets(TestCaseWithFactory):
991 for bmp in source_ref.landing_targets:
992 if bmp.queue_status not in FINAL_STATES:
993 removeSecurityProxy(bmp).deleteProposal()
994- jobs = source_ref.repository.updateLandingTargets([source_ref.path])
995+
996+ # Enable the feature here, since factory.makeBranchMergeProposalForGit
997+ # would also trigger the copy refs call.
998+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
999+ jobs, ref_copies, ref_deletes = (
1000+ source_ref.repository.updateLandingTargets(
1001+ [source_ref.path]))
1002 self.assertEqual(0, len(jobs))
1003+ self.assertEqual(0, len(ref_copies))
1004+ self.assertEqual(0, len(ref_deletes))
1005+ self.assertEqual(0, self.hosting_fixture.copyRefs.call_count)
1006
1007
1008 class TestGitRepositoryMarkRecipesStale(TestCaseWithFactory):
1009diff --git a/lib/lp/code/subscribers/branchmergeproposal.py b/lib/lp/code/subscribers/branchmergeproposal.py
1010index a214c8f..a34ca2a 100644
1011--- a/lib/lp/code/subscribers/branchmergeproposal.py
1012+++ b/lib/lp/code/subscribers/branchmergeproposal.py
1013@@ -1,4 +1,4 @@
1014-# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
1015+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
1016 # GNU Affero General Public License version 3 (see the file LICENSE).
1017
1018 """Event subscribers for branch merge proposals."""
1019@@ -63,9 +63,13 @@ def _trigger_webhook(merge_proposal, payload):
1020 def merge_proposal_created(merge_proposal, event):
1021 """A new merge proposal has been created.
1022
1023- Create a job to update the diff for the merge proposal; trigger webhooks.
1024+ Create a job to update the diff for the merge proposal; trigger webhooks
1025+ and copy virtual refs as needed.
1026 """
1027 getUtility(IUpdatePreviewDiffJobSource).create(merge_proposal)
1028+
1029+ merge_proposal.syncGitVirtualRefs()
1030+
1031 if getFeatureFlag(BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG):
1032 payload = {
1033 "action": "created",
1034@@ -149,3 +153,5 @@ def merge_proposal_deleted(merge_proposal, event):
1035 "old": _compose_merge_proposal_webhook_payload(merge_proposal),
1036 }
1037 _trigger_webhook(merge_proposal, payload)
1038+
1039+ merge_proposal.syncGitVirtualRefs(force_delete=True)
1040diff --git a/lib/lp/code/tests/helpers.py b/lib/lp/code/tests/helpers.py
1041index 4ef843d..3704b06 100644
1042--- a/lib/lp/code/tests/helpers.py
1043+++ b/lib/lp/code/tests/helpers.py
1044@@ -369,6 +369,9 @@ class GitHostingFixture(fixtures.Fixture):
1045 self.getBlob = FakeMethod(result=blob)
1046 self.delete = FakeMethod()
1047 self.disable_memcache = disable_memcache
1048+ self.copyRefs = FakeMethod()
1049+ self.deleteRefs = FakeMethod()
1050+ self.bulkSyncRefs = FakeMethod()
1051
1052 def _setUp(self):
1053 self.useFixture(ZopeUtilityFixture(self, IGitHostingClient))