Launchpad itself

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

  1. Git
  2. lp:~pappacena/launchpad
  3. mp-refs-privacy-change
  4. Merge into 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 (community) 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
Reply
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.

Reply
Revision history for this message
Colin Watson (cjwatson) :
Reply

Unmerged commits

dcdb5f4... by Thiago F. Pappacena

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

f3bda00... by Thiago F. Pappacena

Fixing test

9a2f2f7... by Thiago F. Pappacena

Retry strategy for virtual refs sync job

cca2a0f... by Thiago F. Pappacena

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

2ca86e6... by Thiago F. Pappacena

Fixing test

ebd11bb... by Thiago F. Pappacena

Adding job to reconcile mp virtual refs

3c2ffb2... by Thiago F. Pappacena

Trigger MP reconciliation job when changing repository privacy setting

30c88c7... by Thiago F. Pappacena

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

7e11cb0... by Thiago F. Pappacena

Refactoring virtual refs creation and preparing privacy code

1f7f371... by Thiago F. Pappacena

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 = []