Merge ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad: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.

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))