Merge ~pappacena/launchpad:mp-refs-privacy-change into launchpad:master

Proposed by Thiago F. Pappacena
Status: Needs review
Proposed branch: ~pappacena/launchpad:mp-refs-privacy-change
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:create-mp-refs
Diff against target: 582 lines (+309/-16)
10 files modified
lib/lp/code/configure.zcml (+9/-0)
lib/lp/code/interfaces/gitjob.py (+16/-1)
lib/lp/code/model/branchmergeproposal.py (+9/-7)
lib/lp/code/model/gitjob.py (+63/-1)
lib/lp/code/model/gitrepository.py (+11/-1)
lib/lp/code/model/tests/test_branchmergeproposal.py (+4/-4)
lib/lp/code/model/tests/test_gitjob.py (+127/-1)
lib/lp/code/model/tests/test_gitrepository.py (+62/-0)
lib/lp/services/config/schema-lazr.conf (+4/-0)
lib/lp/testing/fakemethod.py (+4/-1)
Reviewer Review Type Date Requested Status
Colin Watson Needs Information
Review via email: mp+390940@code.launchpad.net

Commit message

Background job to add/remove merge proposal's virtual refs when a repository has its privacy changed

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

There's an unfortunate complication around private-to-public transitions that I fear is going to require quite a bit more work to get right (assuming I haven't got the wrong end of the stick somewhere). Details below.

review: Needs Information
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Added one comment about the complication on private-to-public, just to make sure I understood the scenario. Meanwhile, I'll work out the other details in the MP.

Revision history for this message
Colin Watson (cjwatson) :

Unmerged commits

dcdb5f4... by Thiago F. Pappacena on 2020-09-21

Merge branch 'create-mp-refs' into mp-refs-privacy-change

f3bda00... by Thiago F. Pappacena on 2020-09-17

Fixing test

9a2f2f7... by Thiago F. Pappacena on 2020-09-18

Retry strategy for virtual refs sync job

cca2a0f... by Thiago F. Pappacena on 2020-09-17

Merge branch 'create-mp-refs' into mp-refs-privacy-change

2ca86e6... by Thiago F. Pappacena on 2020-09-17

Fixing test

ebd11bb... by Thiago F. Pappacena on 2020-09-17

Adding job to reconcile mp virtual refs

3c2ffb2... by Thiago F. Pappacena on 2020-09-17

Trigger MP reconciliation job when changing repository privacy setting

30c88c7... by Thiago F. Pappacena on 2020-09-16

Merge branch 'githosting-copy-and-delete-refs' into create-mp-refs

7e11cb0... by Thiago F. Pappacena on 2020-09-16

Refactoring virtual refs creation and preparing privacy code

1f7f371... by Thiago F. Pappacena on 2020-09-11

Adding feature flag and triggering copy on MP creation

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
2index 898e645..4adc8d0 100644
3--- a/lib/lp/code/configure.zcml
4+++ b/lib/lp/code/configure.zcml
5@@ -1111,6 +1111,11 @@
6 provides="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource">
7 <allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource" />
8 </securedutility>
9+ <securedutility
10+ component="lp.code.model.gitjob.GitRepositoryVirtualRefsSyncJob"
11+ provides="lp.code.interfaces.gitjob.IGitRepositoryVirtualRefsSyncJobSource">
12+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryVirtualRefsSyncJobSource" />
13+ </securedutility>
14 <class class="lp.code.model.gitjob.GitRefScanJob">
15 <allow interface="lp.code.interfaces.gitjob.IGitJob" />
16 <allow interface="lp.code.interfaces.gitjob.IGitRefScanJob" />
17@@ -1123,6 +1128,10 @@
18 <allow interface="lp.code.interfaces.gitjob.IGitJob" />
19 <allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJob" />
20 </class>
21+ <class class="lp.code.model.gitjob.GitRepositoryVirtualRefsSyncJob">
22+ <allow interface="lp.code.interfaces.gitjob.IGitJob" />
23+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryVirtualRefsSyncJob" />
24+ </class>
25
26 <lp:help-folder folder="help" name="+help-code" />
27
28diff --git a/lib/lp/code/interfaces/gitjob.py b/lib/lp/code/interfaces/gitjob.py
29index 4f31b19..7c3c038 100644
30--- a/lib/lp/code/interfaces/gitjob.py
31+++ b/lib/lp/code/interfaces/gitjob.py
32@@ -1,4 +1,4 @@
33-# Copyright 2015 Canonical Ltd. This software is licensed under the
34+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
35 # GNU Affero General Public License version 3 (see the file LICENSE).
36
37 """GitJob interfaces."""
38@@ -11,6 +11,8 @@ __all__ = [
39 'IGitRefScanJobSource',
40 'IGitRepositoryModifiedMailJob',
41 'IGitRepositoryModifiedMailJobSource',
42+ 'IGitRepositoryVirtualRefsSyncJob',
43+ 'IGitRepositoryVirtualRefsSyncJobSource',
44 'IReclaimGitRepositorySpaceJob',
45 'IReclaimGitRepositorySpaceJobSource',
46 ]
47@@ -93,3 +95,16 @@ class IGitRepositoryModifiedMailJobSource(IJobSource):
48 :param repository_delta: An `IGitRepositoryDelta` describing the
49 changes.
50 """
51+
52+
53+class IGitRepositoryVirtualRefsSyncJob(IRunnableJob):
54+ """A job to synchronize all MPs virtual refs related to this repository."""
55+
56+
57+class IGitRepositoryVirtualRefsSyncJobSource(IJobSource):
58+
59+ def create(repository):
60+ """Send email about repository modifications.
61+
62+ :param repository: The `IGitRepository` that needs sync.
63+ """
64diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
65index 8a29a99..bd804a8 100644
66--- a/lib/lp/code/model/branchmergeproposal.py
67+++ b/lib/lp/code/model/branchmergeproposal.py
68@@ -1247,7 +1247,7 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
69 self.target_git_repository.getInternalPath(),
70 GIT_MP_VIRTUAL_REF_FORMAT.format(mp=self))]
71
72- def copyGitHostingVirtualRefs(self):
73+ def copyGitHostingVirtualRefs(self, logger=logger):
74 """Requests virtual refs copy operations on GitHosting in order to
75 keep them up-to-date with current MP's state.
76
77@@ -1257,10 +1257,10 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
78 hosting_client = getUtility(IGitHostingClient)
79 hosting_client.copyRefs(
80 self.source_git_repository.getInternalPath(),
81- copy_operations)
82+ copy_operations, logger=logger)
83 return copy_operations
84
85- def deleteGitHostingVirtualRefs(self, except_refs=None):
86+ def deleteGitHostingVirtualRefs(self, except_refs=None, logger=None):
87 """Deletes on git code hosting service all virtual refs, except
88 those ones in the given list."""
89 if self.source_git_ref is None:
90@@ -1278,14 +1278,16 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
91 if ref not in (except_refs or []):
92 hosting_client = getUtility(IGitHostingClient)
93 hosting_client.deleteRef(
94- self.target_git_repository.getInternalPath(), ref)
95+ self.target_git_repository.getInternalPath(), ref,
96+ logger=logger)
97
98- def syncGitHostingVirtualRefs(self):
99+ def syncGitHostingVirtualRefs(self, logger=None):
100 """Requests all copies and deletion of virtual refs to make git code
101 hosting in sync with this MP."""
102- operations = self.copyGitHostingVirtualRefs()
103+ operations = self.copyGitHostingVirtualRefs(logger=logger)
104 copied_refs = [i.target_ref for i in operations]
105- self.deleteGitHostingVirtualRefs(except_refs=copied_refs)
106+ self.deleteGitHostingVirtualRefs(
107+ except_refs=copied_refs, logger=logger)
108
109 def scheduleDiffUpdates(self, return_jobs=True):
110 """See `IBranchMergeProposal`."""
111diff --git a/lib/lp/code/model/gitjob.py b/lib/lp/code/model/gitjob.py
112index 3c041da..d845f59 100644
113--- a/lib/lp/code/model/gitjob.py
114+++ b/lib/lp/code/model/gitjob.py
115@@ -1,4 +1,4 @@
116-# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
117+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
118 # GNU Affero General Public License version 3 (see the file LICENSE).
119
120 __metaclass__ = type
121@@ -45,6 +45,8 @@ from lp.code.interfaces.gitjob import (
122 IGitRefScanJobSource,
123 IGitRepositoryModifiedMailJob,
124 IGitRepositoryModifiedMailJobSource,
125+ IGitRepositoryVirtualRefsSyncJob,
126+ IGitRepositoryVirtualRefsSyncJobSource,
127 IReclaimGitRepositorySpaceJob,
128 IReclaimGitRepositorySpaceJobSource,
129 )
130@@ -100,6 +102,13 @@ class GitJobType(DBEnumeratedType):
131 modifications.
132 """)
133
134+ SYNC_MP_VIRTUAL_REFS = DBItem(3, """
135+ Sync merge proposals virtual refs
136+
137+ This job runs against a repository to synchronize the virtual refs
138+ from all merge proposals related to this repository.
139+ """)
140+
141
142 @implementer(IGitJob)
143 class GitJob(StormBase):
144@@ -404,3 +413,56 @@ class GitRepositoryModifiedMailJob(GitJobDerived):
145 def run(self):
146 """See `IGitRepositoryModifiedMailJob`."""
147 self.getMailer().sendAll()
148+
149+
150+@implementer(IGitRepositoryVirtualRefsSyncJob)
151+@provider(IGitRepositoryVirtualRefsSyncJobSource)
152+class GitRepositoryVirtualRefsSyncJob(GitJobDerived):
153+ """A Job that scans a Git repository for its current list of references."""
154+ class_job_type = GitJobType.SYNC_MP_VIRTUAL_REFS
155+
156+ class RetryException(Exception):
157+ pass
158+
159+ max_retries = 5
160+
161+ retry_error_types = (RetryException, )
162+
163+ config = config.IGitRepositoryVirtualRefsSyncJobSource
164+
165+ @classmethod
166+ def create(cls, repository):
167+ metadata = {"synced_mp_ids": []}
168+ git_job = GitJob(repository, cls.class_job_type, metadata)
169+ job = cls(git_job)
170+ job.celeryRunOnCommit()
171+ return job
172+
173+ @property
174+ def synced_mp_ids(self):
175+ return self.metadata["synced_mp_ids"]
176+
177+ def add_synced_mp_id(self, mp_id):
178+ self.metadata["synced_mp_ids"].append(mp_id)
179+
180+ def run(self):
181+ log.info(
182+ "Starting to re-sync virtual refs from repository %s",
183+ self.repository)
184+ failed = False
185+ for mp in self.repository.landing_targets:
186+ if mp.id in self.synced_mp_ids:
187+ continue
188+ try:
189+ log.info("Re-syncing virtual refs from MP %s", mp)
190+ mp.syncGitHostingVirtualRefs(logger=log)
191+ self.synced_mp_ids.append(mp.id)
192+ except Exception as e:
193+ log.info(
194+ "Re-syncing virtual refs from MP %s failed: %s", mp, e)
195+ failed = True
196+ log.info(
197+ "Finished re-syncing virtual refs from repository %s%s",
198+ self.repository, " with failures" if failed else "")
199+ if failed:
200+ raise GitRepositoryVirtualRefsSyncJob.RetryException()
201diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
202index e2451fa..6379fce 100644
203--- a/lib/lp/code/model/gitrepository.py
204+++ b/lib/lp/code/model/gitrepository.py
205@@ -116,7 +116,10 @@ from lp.code.interfaces.gitcollection import (
206 IGitCollection,
207 )
208 from lp.code.interfaces.githosting import IGitHostingClient
209-from lp.code.interfaces.gitjob import IGitRefScanJobSource
210+from lp.code.interfaces.gitjob import (
211+ IGitRefScanJobSource,
212+ IGitRepositoryVirtualRefsSyncJobSource,
213+ )
214 from lp.code.interfaces.gitlookup import IGitLookup
215 from lp.code.interfaces.gitnamespace import (
216 get_git_namespace,
217@@ -866,6 +869,7 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
218 raise CannotChangeInformationType("Forbidden by project policy.")
219 # XXX cjwatson 2019-03-29: Check privacy rules on snaps that use
220 # this repository.
221+ was_private = self.private
222 self.information_type = information_type
223 self._reconcileAccess()
224 if (information_type in PRIVATE_INFORMATION_TYPES and
225@@ -883,6 +887,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
226 # subscriptions.
227 getUtility(IRemoveArtifactSubscriptionsJobSource).create(user, [self])
228
229+ # If privacy changed, we need to re-sync all virtual refs from
230+ # all MPs to avoid disclosing private code, or to add the virtual
231+ # refs to the now public code.
232+ if was_private != self.private:
233+ getUtility(IGitRepositoryVirtualRefsSyncJobSource).create(self)
234+
235 def setName(self, new_name, user):
236 """See `IGitRepository`."""
237 self.namespace.moveRepository(self, user, new_name=new_name)
238diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
239index 54cf516..277650c 100644
240--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
241+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
242@@ -285,7 +285,7 @@ class TestGitBranchMergeProposalVirtualRefs(TestCaseWithFactory):
243 self.assertEqual(1, self.hosting_fixture.copyRefs.call_count)
244 args, kwargs = self.hosting_fixture.copyRefs.calls[0]
245 arg_path, arg_copy_operations = args
246- self.assertEqual({}, kwargs)
247+ self.assertEqual({'logger': None}, kwargs)
248 self.assertEqual(mp.source_git_repository.getInternalPath(), arg_path)
249 self.assertEqual(1, len(arg_copy_operations))
250 self.assertThat(arg_copy_operations[0], MatchesStructure(
251@@ -357,7 +357,7 @@ class TestGitBranchMergeProposalVirtualRefs(TestCaseWithFactory):
252 # Check lp.code.subscribers.branchmergeproposal.merge_proposal_created.
253 self.assertEqual(1, self.hosting_fixture.copyRefs.call_count)
254 args, kwargs = self.hosting_fixture.copyRefs.calls[0]
255- self.assertEquals({}, kwargs)
256+ self.assertEquals({'logger': None}, kwargs)
257 self.assertEqual(args[0], source_repo.getInternalPath())
258 self.assertEqual(1, len(args[1]))
259 self.assertThat(args[1][0], MatchesStructure(
260@@ -384,7 +384,7 @@ class TestGitBranchMergeProposalVirtualRefs(TestCaseWithFactory):
261 self.assertEqual(0, self.hosting_fixture.copyRefs.call_count)
262 self.assertEqual(1, self.hosting_fixture.deleteRef.call_count)
263 args, kwargs = self.hosting_fixture.deleteRef.calls[0]
264- self.assertEqual({}, kwargs)
265+ self.assertEqual({'logger': None}, kwargs)
266 self.assertEqual(
267 (target_repo.getInternalPath(), "refs/merge/%s/head" % mp.id),
268 args)
269@@ -1658,7 +1658,7 @@ class TestBranchMergeProposalDeletion(TestCaseWithFactory):
270 args = hosting_fixture.deleteRef.calls[0]
271 self.assertEqual((
272 (proposal.target_git_repository.getInternalPath(),
273- 'refs/merge/%s/head' % proposal.id), {}), args)
274+ 'refs/merge/%s/head' % proposal.id), {'logger': None}), args)
275
276
277 class TestBranchMergeProposalBugs(WithVCSScenarios, TestCaseWithFactory):
278diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py
279index 5964944..c92be62 100644
280--- a/lib/lp/code/model/tests/test_gitjob.py
281+++ b/lib/lp/code/model/tests/test_gitjob.py
282@@ -1,4 +1,4 @@
283-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
284+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
285 # GNU Affero General Public License version 3 (see the file LICENSE).
286
287 """Tests for `GitJob`s."""
288@@ -20,13 +20,16 @@ from testtools.matchers import (
289 ContainsDict,
290 Equals,
291 MatchesDict,
292+ MatchesListwise,
293 MatchesSetwise,
294 MatchesStructure,
295 )
296 import transaction
297+from zope.component import getUtility
298 from zope.interface import providedBy
299 from zope.security.proxy import removeSecurityProxy
300
301+from lp.app.enums import InformationType
302 from lp.code.adapters.gitrepository import GitRepositoryDelta
303 from lp.code.enums import (
304 GitGranteeType,
305@@ -38,8 +41,10 @@ from lp.code.interfaces.branchmergeproposal import (
306 from lp.code.interfaces.gitjob import (
307 IGitJob,
308 IGitRefScanJob,
309+ IGitRepositoryVirtualRefsSyncJobSource,
310 IReclaimGitRepositorySpaceJob,
311 )
312+from lp.code.interfaces.gitrepository import GIT_CREATE_MP_VIRTUAL_REF
313 from lp.code.model.gitjob import (
314 describe_repository_delta,
315 GitJob,
316@@ -49,9 +54,11 @@ from lp.code.model.gitjob import (
317 ReclaimGitRepositorySpaceJob,
318 )
319 from lp.code.tests.helpers import GitHostingFixture
320+from lp.services.compat import mock
321 from lp.services.config import config
322 from lp.services.database.constants import UTC_NOW
323 from lp.services.features.testing import FeatureFixture
324+from lp.services.job.interfaces.job import JobStatus
325 from lp.services.job.runner import JobRunner
326 from lp.services.utils import seconds_since_epoch
327 from lp.services.webapp import canonical_url
328@@ -484,5 +491,124 @@ class TestDescribeRepositoryDelta(TestCaseWithFactory):
329 snapshot, repository)
330
331
332+class TestGitRepositoryVirtualRefsSyncJob(TestCaseWithFactory):
333+ """Tests for `GitRepositoryVirtualRefsSyncJob`."""
334+
335+ layer = ZopelessDatabaseLayer
336+
337+ def runJobs(self):
338+ with dbuser("branchscanner"):
339+ job_set = JobRunner.fromReady(
340+ getUtility(IGitRepositoryVirtualRefsSyncJobSource))
341+ job_set.runAll()
342+ return job_set
343+
344+ def test_changing_repo_to_private_deletes_refs(self):
345+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
346+ hosting_fixture = self.useFixture(GitHostingFixture())
347+ mp = self.factory.makeBranchMergeProposalForGit()
348+ source_repo = mp.source_git_repository
349+ target_repo = mp.target_git_repository
350+ source_repo.transitionToInformationType(
351+ InformationType.PRIVATESECURITY, source_repo.owner, False)
352+
353+ hosting_fixture.copyRefs.resetCalls()
354+ hosting_fixture.deleteRef.resetCalls()
355+ jobs = self.runJobs()
356+ self.assertEqual(1, len(jobs.completed_jobs))
357+
358+ self.assertEqual(0, hosting_fixture.copyRefs.call_count)
359+ self.assertEqual(1, hosting_fixture.deleteRef.call_count)
360+ args, kwargs = hosting_fixture.deleteRef.calls[0]
361+ self.assertEqual(
362+ (target_repo.getInternalPath(), 'refs/merge/%s/head' % mp.id),
363+ args)
364+
365+ def test_changing_repo_to_public_recreates_refs(self):
366+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
367+ hosting_fixture = self.useFixture(GitHostingFixture())
368+ mp = self.factory.makeBranchMergeProposalForGit()
369+ source_repo = mp.source_git_repository
370+ source_repo.transitionToInformationType(
371+ InformationType.PRIVATESECURITY, source_repo.owner, False)
372+ self.runJobs()
373+
374+ # Move it back to public.
375+ hosting_fixture.copyRefs.resetCalls()
376+ hosting_fixture.deleteRef.resetCalls()
377+ source_repo.transitionToInformationType(
378+ InformationType.PUBLIC, source_repo.owner, False)
379+ jobs = self.runJobs()
380+ self.assertEqual(1, len(jobs.completed_jobs))
381+
382+ self.assertEqual(1, hosting_fixture.copyRefs.call_count)
383+ self.assertEqual(0, hosting_fixture.deleteRef.call_count)
384+ args, kwargs = hosting_fixture.copyRefs.calls[0]
385+ self.assertEqual({'logger': mock.ANY}, kwargs)
386+ self.assertEqual(2, len(args))
387+ repo, operations = args
388+ self.assertEqual(repo, mp.source_git_repository.getInternalPath())
389+ self.assertThat(operations, MatchesListwise([
390+ MatchesStructure(
391+ source_ref=Equals(mp.source_git_commit_sha1),
392+ target_repo=Equals(mp.target_git_repository.getInternalPath()),
393+ target_ref=Equals("refs/merge/%s/head" % mp.id),
394+ )
395+ ]))
396+
397+ @mock.patch("lp.code.model.branchmergeproposal.BranchMergeProposal."
398+ "syncGitHostingVirtualRefs")
399+ def test_changing_repo_retry_skips_successful_syncs(
400+ self, syncGitHostingVirtualRefs):
401+ # Makes sure that successful syncs are not retried on failures.
402+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
403+ hosting_fixture = self.useFixture(GitHostingFixture())
404+ mp1 = self.factory.makeBranchMergeProposalForGit()
405+ source_repo = mp1.source_git_repository
406+
407+ mp2 = self.factory.makeBranchMergeProposalForGit(
408+ source_ref=mp1.source_git_ref)
409+ mp3 = self.factory.makeBranchMergeProposalForGit(
410+ source_ref=mp1.source_git_ref)
411+
412+ # The 1st call to syncGitHostingVirtualRefs (mp1) should succeed.
413+ # The 2nd call (mp2, first try) should fail
414+ # The 3rd call (mp3) should succeed.
415+ # Then, 4th call (mp2 again) should succeed.
416+ syncGitHostingVirtualRefs.reset_mock()
417+ syncGitHostingVirtualRefs.side_effect = [None, Exception(), None, None]
418+
419+ # Make source repo private, to trigger the job.
420+ hosting_fixture.copyRefs.resetCalls()
421+ hosting_fixture.deleteRef.resetCalls()
422+ source_repo.transitionToInformationType(
423+ InformationType.PRIVATESECURITY, source_repo.owner, False)
424+ jobs = self.runJobs()
425+ # Should have no completed jobs, since the job should be retried.
426+ self.assertEqual(
427+ 0, len(jobs.completed_jobs),
428+ "No job should have been finished.")
429+ self.assertEqual(
430+ 1, len(jobs.incomplete_jobs), "Job retry should be pending.")
431+ self.assertEqual(
432+ 3, syncGitHostingVirtualRefs.call_count,
433+ "Even with a failure on mp2, syncGitHostingVirtualRefs should "
434+ "have been called for every branch merge proposal.")
435+ job = removeSecurityProxy(jobs.incomplete_jobs[0])
436+ self.assertEqual([mp1.id, mp3.id], job.synced_mp_ids)
437+
438+ # Run the job again.
439+ syncGitHostingVirtualRefs.reset_mock()
440+ JobRunner(jobs.incomplete_jobs).runAll()
441+ self.assertEqual(
442+ JobStatus.COMPLETED, jobs.incomplete_jobs[0].status,
443+ "The job should have completed this time, since mp2 sync should "
444+ "not have raised exception.")
445+ self.assertEqual(
446+ 1, syncGitHostingVirtualRefs.call_count,
447+ "mp2.syncGitHostingVirtualRefs should be the only call to this "
448+ "method, since the sync should have been skipped for mp1 and mp3.")
449+ self.assertEqual([mp1.id, mp3.id, mp2.id], job.synced_mp_ids)
450+
451 # XXX cjwatson 2015-03-12: We should test that the jobs work via Celery too,
452 # but that isn't feasible until we have a proper turnip fixture.
453diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
454index b0116e1..c561b3c 100644
455--- a/lib/lp/code/model/tests/test_gitrepository.py
456+++ b/lib/lp/code/model/tests/test_gitrepository.py
457@@ -18,6 +18,7 @@ from datetime import (
458 import email
459 from functools import partial
460 import hashlib
461+import itertools
462 import json
463
464 from breezy import urlutils
465@@ -89,6 +90,7 @@ from lp.code.interfaces.defaultgit import ICanHasDefaultGitRepository
466 from lp.code.interfaces.gitjob import (
467 IGitRefScanJobSource,
468 IGitRepositoryModifiedMailJobSource,
469+ IGitRepositoryVirtualRefsSyncJobSource,
470 )
471 from lp.code.interfaces.gitlookup import IGitLookup
472 from lp.code.interfaces.gitnamespace import (
473@@ -147,6 +149,7 @@ from lp.registry.interfaces.personociproject import IPersonOCIProjectFactory
474 from lp.registry.interfaces.personproduct import IPersonProductFactory
475 from lp.registry.tests.test_accesspolicy import get_policies_for_artifact
476 from lp.services.authserver.xmlrpc import AuthServerAPIView
477+from lp.services.compat import mock
478 from lp.services.config import config
479 from lp.services.database.constants import UTC_NOW
480 from lp.services.database.interfaces import IStore
481@@ -181,6 +184,7 @@ from lp.testing import (
482 verifyObject,
483 )
484 from lp.testing.dbuser import dbuser
485+from lp.testing.fixture import ZopeUtilityFixture
486 from lp.testing.layers import (
487 DatabaseFunctionalLayer,
488 LaunchpadFunctionalLayer,
489@@ -4662,3 +4666,61 @@ class TestGitRepositoryMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
490 ["Caveat check for '%s' failed." %
491 find_caveats_by_name(macaroon2, "lp.expires")[0].caveat_id],
492 issuer, macaroon2, repository, user=repository.owner)
493+
494+
495+class TestGitRepositoryPrivacyChangeSyncVirtualRefs(TestCaseWithFactory):
496+ layer = DatabaseFunctionalLayer
497+
498+ def assertChangePrivacyTriggersSync(
499+ self, from_list, to_list, should_trigger_sync=True):
500+ """Runs repository.transitionToInformationType from every item in
501+ `from_list` to each item in `to_list`, and checks if the virtual
502+ refs sync was triggered or not, depending on `should_trigger_sync`."""
503+ sync_job = mock.Mock()
504+ self.useFixture(ZopeUtilityFixture(
505+ sync_job, IGitRepositoryVirtualRefsSyncJobSource))
506+
507+ admin = self.factory.makeAdministrator()
508+ login_person(admin)
509+ for from_type, to_type in itertools.product(from_list, to_list):
510+ if from_type == to_type:
511+ continue
512+ repository = self.factory.makeGitRepository()
513+ naked_repo = removeSecurityProxy(repository)
514+ naked_repo.information_type = from_type
515+ # Skip access policy reconciliation.
516+ naked_repo._reconcileAccess = mock.Mock()
517+ naked_repo.transitionToInformationType(to_type, admin, False)
518+
519+ if should_trigger_sync:
520+ sync_job.create.assert_called_with(repository)
521+ else:
522+ self.assertEqual(
523+ 0, sync_job.create.call_count,
524+ "Changing from %s to %s should't trigger vrefs sync"
525+ % (from_type, to_type))
526+ sync_job.reset_mock()
527+
528+ def test_setting_repo_public_triggers_ref_sync_job(self):
529+ self.assertChangePrivacyTriggersSync(
530+ PRIVATE_INFORMATION_TYPES,
531+ PUBLIC_INFORMATION_TYPES,
532+ should_trigger_sync=True)
533+
534+ def test_setting_repo_private_triggers_ref_sync_job(self):
535+ self.assertChangePrivacyTriggersSync(
536+ PUBLIC_INFORMATION_TYPES,
537+ PRIVATE_INFORMATION_TYPES,
538+ should_trigger_sync=True)
539+
540+ def test_keeping_repo_private_dont_trigger_ref_sync_job(self):
541+ self.assertChangePrivacyTriggersSync(
542+ PRIVATE_INFORMATION_TYPES,
543+ PRIVATE_INFORMATION_TYPES,
544+ should_trigger_sync=False)
545+
546+ def test_keeping_repo_public_dont_trigger_ref_sync_job(self):
547+ self.assertChangePrivacyTriggersSync(
548+ PUBLIC_INFORMATION_TYPES,
549+ PUBLIC_INFORMATION_TYPES,
550+ should_trigger_sync=False)
551diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
552index bb8afcd..3921da2 100644
553--- a/lib/lp/services/config/schema-lazr.conf
554+++ b/lib/lp/services/config/schema-lazr.conf
555@@ -1828,6 +1828,10 @@ module: lp.code.interfaces.gitjob
556 dbuser: send-branch-mail
557 crontab_group: MAIN
558
559+[IGitRepositoryVirtualRefsSyncJobSource]
560+module: lp.code.interfaces.gitjob
561+dbuser: branchscanner
562+
563 [IInitializeDistroSeriesJobSource]
564 module: lp.soyuz.interfaces.distributionjob
565 dbuser: initializedistroseries
566diff --git a/lib/lp/testing/fakemethod.py b/lib/lp/testing/fakemethod.py
567index 4bba8d3..b4895ce 100644
568--- a/lib/lp/testing/fakemethod.py
569+++ b/lib/lp/testing/fakemethod.py
570@@ -1,4 +1,4 @@
571-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
572+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
573 # GNU Affero General Public License version 3 (see the file LICENSE).
574
575 __metaclass__ = type
576@@ -57,3 +57,6 @@ class FakeMethod:
577 def extract_kwargs(self):
578 """Return just the calls' keyword-arguments dicts."""
579 return [kwargs for args, kwargs in self.calls]
580+
581+ def resetCalls(self):
582+ self.calls = []