Launchpad itself

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

  1. Git
  2. lp:~pappacena/launchpad
  3. git-repo-async-privacy
  4. Merge into master
Proposed by Thiago F. Pappacena
Status: Needs review
Proposed branch: ~pappacena/launchpad:git-repo-async-privacy
Merge into: launchpad:master
Diff against target: 734 lines (+312/-31)
17 files modified
database/schema/security.cfg (+18/-0)
lib/lp/app/widgets/itemswidgets.py (+6/-2)
lib/lp/code/browser/gitrepository.py (+9/-1)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+1/-1)
lib/lp/code/browser/tests/test_gitrepository.py (+69/-3)
lib/lp/code/configure.zcml (+9/-0)
lib/lp/code/interfaces/gitjob.py (+20/-1)
lib/lp/code/interfaces/gitrepository.py (+4/-0)
lib/lp/code/model/gitjob.py (+57/-0)
lib/lp/code/model/gitref.py (+1/-1)
lib/lp/code/model/gitrepository.py (+42/-9)
lib/lp/code/model/tests/test_gitcollection.py (+1/-1)
lib/lp/code/model/tests/test_gitjob.py (+59/-0)
lib/lp/code/model/tests/test_gitrepository.py (+4/-6)
lib/lp/code/xmlrpc/tests/test_git.py (+5/-5)
lib/lp/services/config/schema-lazr.conf (+6/-0)
lib/lp/testing/factory.py (+1/-1)
Reviewer Review Type Date Requested Status
Colin Watson (community) Needs Information
Ioana Lasc (community) Approve
Review via email: mp+392776@code.launchpad.net

Commit message

Doing repository privacy change in background, and blocking user from changing it while another change is in progress.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

This MP is already quite big, so I'll create in another MP the garbo job that will deal with GitRepositories that are PENDING_INFORMATION_TYPE_TRANSITION for too long (to avoid blocking them forever).

Reply
Revision history for this message
Ioana Lasc (ilasc) wrote :

LGTM

review: Approve
Reply
~pappacena/launchpad:git-repo-async-privacy updated
974b22a... by Thiago F. Pappacena

Fixing test

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

I think this is mostly OK, but I wonder if it could be even better. Instead of adding a new item to GitRepositoryStatus, have you considered just checking whether there's a pending job of the appropriate type, a bit like GitRepository.pending_updates? That would have the advantage that there'd be no need for a garbo job (https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/392809) to clean things up if the information-type-changing job fails: it would just automatically become available again, which feels like a cleaner design.

review: Needs Information
Reply
~pappacena/launchpad:git-repo-async-privacy updated
826cbad... by Thiago F. Pappacena

Encapsulating GitRepository.pending_change_information_type

eeeb231... by Thiago F. Pappacena

Renaming GitRepo information type change job

2493c1d... by Thiago F. Pappacena

Changing exception msg when blocking info type transition for git repo

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

Pushed the requested changes.

I thought about just checking on-the-fly for unfinished jobs, but didn't go that way because it could be slightly expensive to run such query. But we do that in so few places that I agree it doesn't worth to concentrate that check in a garbo job. I have changed it too.

Reply

Unmerged commits

2493c1d... by Thiago F. Pappacena

Changing exception msg when blocking info type transition for git repo

eeeb231... by Thiago F. Pappacena

Renaming GitRepo information type change job

826cbad... by Thiago F. Pappacena

Encapsulating GitRepository.pending_change_information_type

974b22a... by Thiago F. Pappacena

Fixing test

4bd7dcc... by Thiago F. Pappacena

Fixing configuration of info type transition job

76e0272... by Thiago F. Pappacena

Blocking at the UI information type change while it is already changing

caf2673... by Thiago F. Pappacena

Adding tests for async GitRepo.transitionToInformationType

3a5e053... by Thiago F. Pappacena

Changing GitRepo.transitionInformationType method to be async

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 11aba6f..290e7bc 100644
3--- a/database/schema/security.cfg
4+++ b/database/schema/security.cfg
5@@ -2082,6 +2082,24 @@ public.webhookjob = SELECT, INSERT
6 public.xref = SELECT, INSERT, DELETE
7 type=user
8
9+[privacy-change-jobs]
10+groups=script
11+public.accessartifact = SELECT, UPDATE, DELETE, INSERT
12+public.accessartifactgrant = SELECT, UPDATE, DELETE, INSERT
13+public.accesspolicyartifact = SELECT, UPDATE, DELETE, INSERT
14+public.accesspolicygrant = SELECT, UPDATE, DELETE
15+public.account = SELECT
16+public.distribution = SELECT
17+public.gitjob = SELECT, UPDATE
18+public.gitrepository = SELECT, UPDATE
19+public.gitsubscription = SELECT, UPDATE, DELETE
20+public.job = SELECT, INSERT, UPDATE
21+public.person = SELECT
22+public.product = SELECT
23+public.sharingjob = SELECT, INSERT, UPDATE
24+public.teamparticipation = SELECT
25+type=user
26+
27 [sharing-jobs]
28 groups=script
29 public.accessartifactgrant = SELECT, UPDATE, DELETE
30diff --git a/lib/lp/app/widgets/itemswidgets.py b/lib/lp/app/widgets/itemswidgets.py
31index 1dbb59f..a644a96 100644
32--- a/lib/lp/app/widgets/itemswidgets.py
33+++ b/lib/lp/app/widgets/itemswidgets.py
34@@ -189,25 +189,29 @@ class LaunchpadRadioWidgetWithDescription(LaunchpadRadioWidget):
35 """Render an item of the list."""
36 text = html_escape(text)
37 id = '%s.%s' % (name, index)
38+ extra_attr = {"disabled": "disabled"} if self.context.readonly else {}
39 elem = renderElement(u'input',
40 value=value,
41 name=name,
42 id=id,
43 cssClass=cssClass,
44- type='radio')
45+ type='radio',
46+ **extra_attr)
47 return self._renderRow(text, value, id, elem)
48
49 def renderSelectedItem(self, index, text, value, name, cssClass):
50 """Render a selected item of the list."""
51 text = html_escape(text)
52 id = '%s.%s' % (name, index)
53+ extra_attr = {"disabled": "disabled"} if self.context.readonly else {}
54 elem = renderElement(u'input',
55 value=value,
56 name=name,
57 id=id,
58 cssClass=cssClass,
59 checked="checked",
60- type='radio')
61+ type='radio',
62+ **extra_attr)
63 return self._renderRow(text, value, id, elem)
64
65 def renderExtraHint(self):
66diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
67index 2552ffb..1ec1b16 100644
68--- a/lib/lp/code/browser/gitrepository.py
69+++ b/lib/lp/code/browser/gitrepository.py
70@@ -483,6 +483,8 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
71 def warning_message(self):
72 if self.context.status == GitRepositoryStatus.CREATING:
73 return "This repository is being created."
74+ if self.context.pending_change_information_type:
75+ return "This repository's information type is being changed."
76 return None
77
78 @property
79@@ -573,6 +575,7 @@ class GitRepositoryEditFormView(LaunchpadEditFormView):
80 @cachedproperty
81 def schema(self):
82 info_types = self.getInformationTypesToShow()
83+ read_only_info_type = self.context.pending_change_information_type
84
85 class GitRepositoryEditSchema(Interface):
86 """Defines the fields for the edit form.
87@@ -582,7 +585,8 @@ class GitRepositoryEditFormView(LaunchpadEditFormView):
88 """
89 use_template(IGitRepository, include=["default_branch"])
90 information_type = copy_field(
91- IGitRepository["information_type"], readonly=False,
92+ IGitRepository["information_type"],
93+ readonly=read_only_info_type,
94 vocabulary=InformationTypeVocabulary(types=info_types))
95 name = copy_field(IGitRepository["name"], readonly=False)
96 owner = copy_field(IGitRepository["owner"], readonly=False)
97@@ -785,6 +789,10 @@ class GitRepositoryEditView(CodeEditOwnerMixin, GitRepositoryEditFormView):
98 self.widgets["target"].hint = (
99 "This is the default repository for this target, so it "
100 "cannot be moved to another target.")
101+ if self.context.pending_change_information_type:
102+ self.widgets["information_type"].hint = (
103+ "The information type is being changed. The operation needs "
104+ "to finish before you can change it again.")
105 if self.context.default_branch:
106 self.widgets['default_branch'].context.required = True
107
108diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
109index f2ec4ec..83c51dc 100644
110--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
111+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
112@@ -2336,7 +2336,7 @@ class TestLatestProposalsForEachBranchGit(
113
114 @staticmethod
115 def _setBranchInvisible(branch):
116- removeSecurityProxy(branch.repository).transitionToInformationType(
117+ removeSecurityProxy(branch.repository)._transitionToInformationType(
118 InformationType.USERDATA, branch.owner, verify_policy=False)
119
120
121diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
122index 262d038..13a3a16 100644
123--- a/lib/lp/code/browser/tests/test_gitrepository.py
124+++ b/lib/lp/code/browser/tests/test_gitrepository.py
125@@ -60,6 +60,9 @@ from lp.code.enums import (
126 GitRepositoryType,
127 )
128 from lp.code.interfaces.gitcollection import IGitCollection
129+from lp.code.interfaces.gitjob import (
130+ IGitRepositoryChangeInformationTypeJobSource,
131+ )
132 from lp.code.interfaces.gitrepository import IGitRepositorySet
133 from lp.code.interfaces.revision import IRevisionSet
134 from lp.code.model.gitjob import GitRefScanJob
135@@ -161,6 +164,15 @@ class TestGitRepositoryView(BrowserTestCase):
136 self.assertTextMatchesExpressionIgnoreWhitespace(
137 r"""This repository is being created\..*""", text)
138
139+ def test_changing_info_type_warning_message_is_present(self):
140+ repository = removeSecurityProxy(self.factory.makeGitRepository())
141+ repository.transitionToInformationType(
142+ InformationType.PRIVATESECURITY, repository.owner)
143+ text = self.getMainText(repository, "+index", user=repository.owner)
144+ self.assertTextMatchesExpressionIgnoreWhitespace(
145+ r"""This repository's information type is being changed\..*""",
146+ text)
147+
148 def test_creating_warning_message_is_not_shown(self):
149 repository = removeSecurityProxy(self.factory.makeGitRepository())
150 repository.status = GitRepositoryStatus.AVAILABLE
151@@ -1185,8 +1197,54 @@ class TestGitRepositoryEditView(TestCaseWithFactory):
152 browser.getControl("Private", index=1).click()
153 browser.getControl("Change Git Repository").click()
154 with person_logged_in(person):
155- self.assertEqual(
156- InformationType.USERDATA, repository.information_type)
157+ self.assertTrue(repository.pending_change_information_type)
158+ job_util = getUtility(
159+ IGitRepositoryChangeInformationTypeJobSource)
160+ jobs = list(job_util.iterReady())
161+ self.assertEqual(1, len(jobs))
162+ job = removeSecurityProxy(jobs[0])
163+ self.assertEqual(repository, job.repository)
164+ self.assertEqual(InformationType.USERDATA, job.information_type)
165+ self.assertEqual(admin, job.user)
166+
167+ def test_information_type_in_ui_blocked_if_already_changing(self):
168+ # The information_type of a repository can't be changed via the UI
169+ # if the repository is already pending a info type change.
170+ person = self.factory.makePerson()
171+ repository = self.factory.makeGitRepository(owner=person)
172+ with admin_logged_in():
173+ repository.transitionToInformationType(
174+ InformationType.PRIVATESECURITY, repository.owner)
175+ # There should be 1 job pending to be executed.
176+ job_util = getUtility(
177+ IGitRepositoryChangeInformationTypeJobSource)
178+ jobs = list(job_util.iterReady())
179+ self.assertEqual(1, len(jobs))
180+ admin = getUtility(ILaunchpadCelebrities).admin.teamowner
181+ browser = self.getUserBrowser(
182+ canonical_url(repository) + "/+edit", user=admin)
183+ # Make sure the privacy controls are all disabled in the UI.
184+ controls = [
185+ "Public", "Public Security", "Private Security", "Private",
186+ "Proprietary", "Embargoed"]
187+ self.assertTrue(
188+ all(browser.getControl(i, index=0).disabled for i in controls))
189+ expected_msg = (
190+ "The information type is being changed. The operation needs to "
191+ "finish before you can change it again.")
192+ self.assertIn(expected_msg, extract_text(browser.contents))
193+
194+ # Trying to change should have no effect in the backend, since the
195+ # repository is already changing info type and this field is read-only.
196+ browser.getControl("Private", index=1).click()
197+ browser.getControl("Change Git Repository").click()
198+ with person_logged_in(person):
199+ self.assertTrue(repository.pending_change_information_type)
200+ # It should have the same job it already had.
201+ job_util = getUtility(
202+ IGitRepositoryChangeInformationTypeJobSource)
203+ new_jobs = list(job_util.iterReady())
204+ self.assertEqual(jobs, new_jobs)
205
206 def test_edit_view_ajax_render(self):
207 # An information type change request is processed as expected when
208@@ -1208,8 +1266,16 @@ class TestGitRepositoryEditView(TestCaseWithFactory):
209 person, repository.target, repository, view]
210 result = view.render()
211 self.assertEqual("", result)
212+ self.assertTrue(repository.pending_change_information_type)
213+ job_util = getUtility(
214+ IGitRepositoryChangeInformationTypeJobSource)
215+ jobs = list(job_util.iterReady())
216+ self.assertEqual(1, len(jobs))
217+ job = removeSecurityProxy(jobs[0])
218+ self.assertEqual(repository, job.repository)
219 self.assertEqual(
220- repository.information_type, InformationType.PUBLICSECURITY)
221+ InformationType.PUBLICSECURITY, job.information_type)
222+ self.assertEqual(person, job.user)
223
224 def test_change_default_branch(self):
225 # An authorised user can change the default branch to one that
226diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
227index 898e645..3b03c7d 100644
228--- a/lib/lp/code/configure.zcml
229+++ b/lib/lp/code/configure.zcml
230@@ -1111,6 +1111,11 @@
231 provides="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource">
232 <allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource" />
233 </securedutility>
234+ <securedutility
235+ component="lp.code.model.gitjob.GitRepositoryChangeInformationTypeJob"
236+ provides="lp.code.interfaces.gitjob.IGitRepositoryChangeInformationTypeJobSource">
237+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryChangeInformationTypeJobSource" />
238+ </securedutility>
239 <class class="lp.code.model.gitjob.GitRefScanJob">
240 <allow interface="lp.code.interfaces.gitjob.IGitJob" />
241 <allow interface="lp.code.interfaces.gitjob.IGitRefScanJob" />
242@@ -1123,6 +1128,10 @@
243 <allow interface="lp.code.interfaces.gitjob.IGitJob" />
244 <allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJob" />
245 </class>
246+ <class class="lp.code.model.gitjob.GitRepositoryChangeInformationTypeJob">
247+ <allow interface="lp.code.interfaces.gitjob.IGitJob" />
248+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryChangeInformationTypeJob" />
249+ </class>
250
251 <lp:help-folder folder="help" name="+help-code" />
252
253diff --git a/lib/lp/code/interfaces/gitjob.py b/lib/lp/code/interfaces/gitjob.py
254index 4f31b19..4bbca04 100644
255--- a/lib/lp/code/interfaces/gitjob.py
256+++ b/lib/lp/code/interfaces/gitjob.py
257@@ -1,4 +1,4 @@
258-# Copyright 2015 Canonical Ltd. This software is licensed under the
259+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
260 # GNU Affero General Public License version 3 (see the file LICENSE).
261
262 """GitJob interfaces."""
263@@ -11,6 +11,8 @@ __all__ = [
264 'IGitRefScanJobSource',
265 'IGitRepositoryModifiedMailJob',
266 'IGitRepositoryModifiedMailJobSource',
267+ 'IGitRepositoryChangeInformationTypeJob',
268+ 'IGitRepositoryChangeInformationTypeJobSource',
269 'IReclaimGitRepositorySpaceJob',
270 'IReclaimGitRepositorySpaceJobSource',
271 ]
272@@ -93,3 +95,20 @@ class IGitRepositoryModifiedMailJobSource(IJobSource):
273 :param repository_delta: An `IGitRepositoryDelta` describing the
274 changes.
275 """
276+
277+
278+class IGitRepositoryChangeInformationTypeJob(IRunnableJob):
279+ """A Job to change repository's information type."""
280+
281+
282+class IGitRepositoryChangeInformationTypeJobSource(IJobSource):
283+
284+ def create(repository, user, information_type, verify_policy=True):
285+ """Create a job to change git repository's information type.
286+
287+ :param repository: The `IGitRepository` that was modified.
288+ :param information_type: The `InformationType` to transition to.
289+ :param user: The `IPerson` who is making the change.
290+ :param verify_policy: Check if the new information type complies
291+ with the `IGitNamespacePolicy`.
292+ """
293diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
294index 192133a..dac0e8f 100644
295--- a/lib/lp/code/interfaces/gitrepository.py
296+++ b/lib/lp/code/interfaces/gitrepository.py
297@@ -582,6 +582,10 @@ class IGitRepositoryView(IHasRecipes):
298 "Whether there are recent changes in this repository that have not "
299 "yet been scanned.")
300
301+ pending_change_information_type = Attribute(
302+ "Whether there is a pending request to change the information type "
303+ "of this repository that have not been processed yet.")
304+
305 def updateMergeCommitIDs(paths):
306 """Update commit SHA1s of merge proposals for this repository.
307
308diff --git a/lib/lp/code/model/gitjob.py b/lib/lp/code/model/gitjob.py
309index 1eb87da..f1e3d2a 100644
310--- a/lib/lp/code/model/gitjob.py
311+++ b/lib/lp/code/model/gitjob.py
312@@ -33,10 +33,12 @@ from zope.interface import (
313 provider,
314 )
315
316+from lp.app.enums import InformationType
317 from lp.app.errors import NotFoundError
318 from lp.code.enums import (
319 GitActivityType,
320 GitPermissionType,
321+ GitRepositoryStatus,
322 )
323 from lp.code.interfaces.githosting import IGitHostingClient
324 from lp.code.interfaces.gitjob import (
325@@ -45,6 +47,8 @@ from lp.code.interfaces.gitjob import (
326 IGitRefScanJobSource,
327 IGitRepositoryModifiedMailJob,
328 IGitRepositoryModifiedMailJobSource,
329+ IGitRepositoryChangeInformationTypeJob,
330+ IGitRepositoryChangeInformationTypeJobSource,
331 IReclaimGitRepositorySpaceJob,
332 IReclaimGitRepositorySpaceJobSource,
333 )
334@@ -100,6 +104,13 @@ class GitJobType(DBEnumeratedType):
335 modifications.
336 """)
337
338+ REPOSITORY_TRANSITION_TO_INFO_TYPE = DBItem(3, """
339+ Change repository's information type
340+
341+ This job runs when a user requests to change privacy settings of a
342+ repository.
343+ """)
344+
345
346 @implementer(IGitJob)
347 class GitJob(StormBase):
348@@ -393,3 +404,49 @@ class GitRepositoryModifiedMailJob(GitJobDerived):
349 def run(self):
350 """See `IGitRepositoryModifiedMailJob`."""
351 self.getMailer().sendAll()
352+
353+
354+@implementer(IGitRepositoryChangeInformationTypeJob)
355+@provider(IGitRepositoryChangeInformationTypeJobSource)
356+class GitRepositoryChangeInformationTypeJob(GitJobDerived):
357+ """A Job to change git repository's information type."""
358+
359+ class_job_type = GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE
360+
361+ config = config.IGitRepositoryChangeInformationTypeJobSource
362+
363+ @classmethod
364+ def create(cls, repository, information_type, user, verify_policy=True):
365+ """See `IGitRepositoryChangeInformationTypeJobSource`."""
366+ metadata = {
367+ "user": user.id,
368+ "information_type": information_type.value,
369+ "verify_policy": verify_policy,
370+ }
371+ git_job = GitJob(repository, cls.class_job_type, metadata)
372+ job = cls(git_job)
373+ job.celeryRunOnCommit()
374+ return job
375+
376+ @property
377+ def user(self):
378+ return getUtility(IPersonSet).get(self.metadata["user"])
379+
380+ @property
381+ def verify_policy(self):
382+ return self.metadata["verify_policy"]
383+
384+ @property
385+ def information_type(self):
386+ return InformationType.items[self.metadata["information_type"]]
387+
388+ def run(self):
389+ """See `IGitRepositoryChangeInformationTypeJob`."""
390+ if not self.repository.pending_change_information_type:
391+ raise AttributeError(
392+ "The repository %s is not pending information type change." %
393+ self.repository)
394+
395+ self.repository._transitionToInformationType(
396+ self.information_type, self.user, self.verify_policy)
397+ self.repository.status = GitRepositoryStatus.AVAILABLE
398diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
399index f7dc142..e2f788d 100644
400--- a/lib/lp/code/model/gitref.py
401+++ b/lib/lp/code/model/gitref.py
402@@ -180,7 +180,7 @@ class GitRefMixin:
403
404 def transitionToInformationType(self, information_type, user,
405 verify_policy=True):
406- return self.repository.transitionToInformationType(
407+ return self.repository._transitionToInformationType(
408 information_type, user, verify_policy=verify_policy)
409
410 @property
411diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
412index fb5e703..d5bd382 100644
413--- a/lib/lp/code/model/gitrepository.py
414+++ b/lib/lp/code/model/gitrepository.py
415@@ -117,7 +117,10 @@ from lp.code.interfaces.gitcollection import (
416 IGitCollection,
417 )
418 from lp.code.interfaces.githosting import IGitHostingClient
419-from lp.code.interfaces.gitjob import IGitRefScanJobSource
420+from lp.code.interfaces.gitjob import (
421+ IGitRefScanJobSource,
422+ IGitRepositoryChangeInformationTypeJobSource,
423+ )
424 from lp.code.interfaces.gitlookup import IGitLookup
425 from lp.code.interfaces.gitnamespace import (
426 get_git_namespace,
427@@ -882,6 +885,28 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
428 def transitionToInformationType(self, information_type, user,
429 verify_policy=True):
430 """See `IGitRepository`."""
431+ error_msg = None
432+ if self.status != GitRepositoryStatus.AVAILABLE:
433+ error_msg = ("Cannot change privacy settings while git "
434+ "repository is being created.")
435+ elif self.pending_change_information_type:
436+ error_msg = ("Cannot change privacy settings while git repository "
437+ "is pending another information type change.")
438+ if error_msg is not None:
439+ raise CannotChangeInformationType(error_msg)
440+ util = getUtility(
441+ IGitRepositoryChangeInformationTypeJobSource)
442+ return util.create(self, information_type, user, verify_policy)
443+
444+ def _transitionToInformationType(self, information_type, user,
445+ verify_policy=True):
446+ """Synchronously make the change in this repository's information
447+ type.
448+
449+ External callers should use the async, public version of this
450+ method, since it deals with the side effects of changing
451+ repository's privacy changes.
452+ """
453 if self.information_type == information_type:
454 return
455 if (verify_policy and
456@@ -1120,21 +1145,29 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
457 """See `IGitRepository`."""
458 return self.namespace.areRepositoriesMergeable(self, other)
459
460- @property
461- def pending_updates(self):
462- """See `IGitRepository`."""
463- from lp.code.model.gitjob import (
464- GitJob,
465- GitJobType,
466- )
467+ def hasPendingJob(self, job_type):
468+ from lp.code.model.gitjob import GitJob
469 jobs = Store.of(self).find(
470 GitJob,
471 GitJob.repository == self,
472- GitJob.job_type == GitJobType.REF_SCAN,
473+ GitJob.job_type == job_type,
474 GitJob.job == Job.id,
475 Job._status.is_in([JobStatus.WAITING, JobStatus.RUNNING]))
476 return not jobs.is_empty()
477
478+ @property
479+ def pending_updates(self):
480+ """See `IGitRepository`."""
481+ from lp.code.model.gitjob import GitJobType
482+ return self.hasPendingJob(GitJobType.REF_SCAN)
483+
484+ @property
485+ def pending_change_information_type(self):
486+ """See `IGitRepository`."""
487+ from lp.code.model.gitjob import GitJobType
488+ return self.hasPendingJob(
489+ GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE)
490+
491 def updateMergeCommitIDs(self, paths):
492 """See `IGitRepository`."""
493 store = Store.of(self)
494diff --git a/lib/lp/code/model/tests/test_gitcollection.py b/lib/lp/code/model/tests/test_gitcollection.py
495index 447fcb4..372a633 100644
496--- a/lib/lp/code/model/tests/test_gitcollection.py
497+++ b/lib/lp/code/model/tests/test_gitcollection.py
498@@ -599,7 +599,7 @@ class TestBranchMergeProposals(TestCaseWithFactory):
499 registrant = self.factory.makePerson()
500 mp1 = self.factory.makeBranchMergeProposalForGit(registrant=registrant)
501 naked_repository = removeSecurityProxy(mp1.target_git_repository)
502- naked_repository.transitionToInformationType(
503+ naked_repository._transitionToInformationType(
504 InformationType.USERDATA, registrant, verify_policy=False)
505 collection = self.all_repositories.visibleByUser(None)
506 proposals = collection.getMergeProposals()
507diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py
508index 3f606c0..b4487b9 100644
509--- a/lib/lp/code/model/tests/test_gitjob.py
510+++ b/lib/lp/code/model/tests/test_gitjob.py
511@@ -25,13 +25,16 @@ from testtools.matchers import (
512 MatchesStructure,
513 )
514 import transaction
515+from zope.component import getUtility
516 from zope.interface import providedBy
517 from zope.security.proxy import removeSecurityProxy
518
519+from lp.app.enums import InformationType
520 from lp.code.adapters.gitrepository import GitRepositoryDelta
521 from lp.code.enums import (
522 GitGranteeType,
523 GitObjectType,
524+ GitRepositoryStatus,
525 )
526 from lp.code.interfaces.branchmergeproposal import (
527 BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG,
528@@ -39,6 +42,7 @@ from lp.code.interfaces.branchmergeproposal import (
529 from lp.code.interfaces.gitjob import (
530 IGitJob,
531 IGitRefScanJob,
532+ IGitRepositoryChangeInformationTypeJobSource,
533 IReclaimGitRepositorySpaceJob,
534 )
535 from lp.code.model.gitjob import (
536@@ -47,9 +51,11 @@ from lp.code.model.gitjob import (
537 GitJobDerived,
538 GitJobType,
539 GitRefScanJob,
540+ GitRepositoryChangeInformationTypeJob,
541 ReclaimGitRepositorySpaceJob,
542 )
543 from lp.code.tests.helpers import GitHostingFixture
544+from lp.registry.errors import CannotChangeInformationType
545 from lp.services.config import config
546 from lp.services.database.constants import UTC_NOW
547 from lp.services.features.testing import FeatureFixture
548@@ -362,6 +368,59 @@ class TestReclaimGitRepositorySpaceJob(TestCaseWithFactory):
549 self.assertEqual([(path,)], hosting_fixture.delete.extract_args())
550
551
552+class TestGitRepositoryTransitionInformationType(TestCaseWithFactory):
553+
554+ layer = ZopelessDatabaseLayer
555+
556+ def test_block_multiple_requests_to_change_info_type(self):
557+ repo = self.factory.makeGitRepository()
558+ repo.transitionToInformationType(
559+ InformationType.PRIVATESECURITY, repo.owner)
560+ self.assertTrue(repo.pending_change_information_type)
561+ expected_msg = (
562+ "Cannot change privacy settings while git repository is "
563+ "pending another information type change.")
564+ self.assertRaisesRegex(
565+ CannotChangeInformationType, expected_msg,
566+ repo.transitionToInformationType,
567+ InformationType.PROPRIETARY, repo.owner)
568+
569+ def test_avoid_transitioning_while_creating(self):
570+ repo = self.factory.makeGitRepository()
571+ removeSecurityProxy(repo).status = GitRepositoryStatus.CREATING
572+ expected_msg = (
573+ "Cannot change privacy settings while git repository is "
574+ "being created.")
575+ self.assertRaisesRegex(
576+ CannotChangeInformationType, expected_msg,
577+ repo.transitionToInformationType,
578+ InformationType.PROPRIETARY, repo.owner)
579+
580+ def test_run_changes_info_type(self):
581+ repo = self.factory.makeGitRepository(
582+ information_type=InformationType.PUBLIC)
583+ # Change to a private info type and with verify_policy, so we hit as
584+ # many database tables as possible.
585+ repo.transitionToInformationType(
586+ InformationType.PRIVATESECURITY, repo.owner, verify_policy=True)
587+ self.assertTrue(repo.pending_change_information_type)
588+
589+ job_util = getUtility(
590+ IGitRepositoryChangeInformationTypeJobSource)
591+ jobs = list(job_util.iterReady())
592+ self.assertEqual(1, len(jobs))
593+ with dbuser(GitRepositoryChangeInformationTypeJob.config.dbuser):
594+ JobRunner(jobs).runAll()
595+
596+ self.assertEqual(GitRepositoryStatus.AVAILABLE, repo.status)
597+ self.assertEqual(
598+ InformationType.PRIVATESECURITY, repo.information_type)
599+
600+ # After the job finished, another change is possible.
601+ repo.transitionToInformationType(InformationType.PUBLIC, repo.owner)
602+ self.assertTrue(repo.pending_change_information_type)
603+
604+
605 class TestDescribeRepositoryDelta(TestCaseWithFactory):
606 """Tests for `describe_repository_delta`."""
607
608diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
609index b83f9da..f0ba2b4 100644
610--- a/lib/lp/code/model/tests/test_gitrepository.py
611+++ b/lib/lp/code/model/tests/test_gitrepository.py
612@@ -809,7 +809,7 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
613
614 def test_private_subscription_does_not_disable_deletion(self):
615 # A private repository that has a subscription can be deleted.
616- self.repository.transitionToInformationType(
617+ removeSecurityProxy(self.repository)._transitionToInformationType(
618 InformationType.USERDATA, self.repository.owner,
619 verify_policy=False)
620 self.repository.subscribe(
621@@ -2054,8 +2054,7 @@ class TestGitRepositoryModerate(TestCaseWithFactory):
622 project.setBranchSharingPolicy(BranchSharingPolicy.PUBLIC)
623 repository.transitionToInformationType(
624 InformationType.PRIVATESECURITY, project.owner)
625- self.assertEqual(
626- InformationType.PRIVATESECURITY, repository.information_type)
627+ self.assertTrue(repository.pending_change_information_type)
628
629 def test_attribute_smoketest(self):
630 # Users with launchpad.Moderate can set attributes.
631@@ -3454,7 +3453,7 @@ class TestGitRepositorySet(TestCaseWithFactory):
632 ]
633 for repository, modified_date in zip(repositories, modified_dates):
634 removeSecurityProxy(repository).date_last_modified = modified_date
635- removeSecurityProxy(repositories[0]).transitionToInformationType(
636+ removeSecurityProxy(repositories[0])._transitionToInformationType(
637 InformationType.PRIVATESECURITY, repositories[0].registrant)
638 self.assertEqual(
639 [repositories[3], repositories[4], repositories[1],
640@@ -3966,8 +3965,7 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
641 json.dumps({"information_type": "Public Security"}))
642 self.assertEqual(209, response.status)
643 with person_logged_in(ANONYMOUS):
644- self.assertEqual(
645- InformationType.PUBLICSECURITY, repository_db.information_type)
646+ self.assertTrue(repository_db.pending_change_information_type)
647
648 def test_set_information_type_other_person(self):
649 # An unrelated user cannot change the information type.
650diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
651index dcbcee6..e884cd2 100644
652--- a/lib/lp/code/xmlrpc/tests/test_git.py
653+++ b/lib/lp/code/xmlrpc/tests/test_git.py
654@@ -899,7 +899,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
655 for _ in range(2)]
656 private_repository = code_imports[0].git_repository
657 removeSecurityProxy(
658- private_repository).transitionToInformationType(
659+ private_repository)._transitionToInformationType(
660 InformationType.PRIVATESECURITY, private_repository.owner)
661 with celebrity_logged_in("vcs_imports"):
662 jobs = [
663@@ -1077,7 +1077,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
664 for _ in range(2)]
665 private_repository = code_imports[0].git_repository
666 removeSecurityProxy(
667- private_repository).transitionToInformationType(
668+ private_repository)._transitionToInformationType(
669 InformationType.PRIVATESECURITY, private_repository.owner)
670 with celebrity_logged_in("vcs_imports"):
671 jobs = [
672@@ -1687,7 +1687,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
673 target_rcs_type=TargetRevisionControlSystems.GIT)
674 for _ in range(2)]
675 private_repository = code_imports[0].git_repository
676- removeSecurityProxy(private_repository).transitionToInformationType(
677+ removeSecurityProxy(private_repository)._transitionToInformationType(
678 InformationType.PRIVATESECURITY, private_repository.owner)
679 with celebrity_logged_in("vcs_imports"):
680 jobs = [
681@@ -2012,7 +2012,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
682 target_rcs_type=TargetRevisionControlSystems.GIT)
683 for _ in range(2)]
684 private_repository = code_imports[0].git_repository
685- removeSecurityProxy(private_repository).transitionToInformationType(
686+ removeSecurityProxy(private_repository)._transitionToInformationType(
687 InformationType.PRIVATESECURITY, private_repository.owner)
688 with celebrity_logged_in("vcs_imports"):
689 jobs = [
690@@ -2268,7 +2268,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
691 target_rcs_type=TargetRevisionControlSystems.GIT)
692 for _ in range(2)]
693 private_repository = code_imports[0].git_repository
694- removeSecurityProxy(private_repository).transitionToInformationType(
695+ removeSecurityProxy(private_repository)._transitionToInformationType(
696 InformationType.PRIVATESECURITY, private_repository.owner)
697 with celebrity_logged_in("vcs_imports"):
698 jobs = [
699diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
700index aaa9d30..1157cb5 100644
701--- a/lib/lp/services/config/schema-lazr.conf
702+++ b/lib/lp/services/config/schema-lazr.conf
703@@ -1768,6 +1768,7 @@ job_sources:
704 ICommercialExpiredJobSource,
705 IExpiringMembershipNotificationJobSource,
706 IGitRepositoryModifiedMailJobSource,
707+ IGitRepositoryChangeInformationTypeJobSource,
708 IMembershipNotificationJobSource,
709 IOCIRecipeRequestBuildsJobSource,
710 IOCIRegistryUploadJobSource,
711@@ -1829,6 +1830,11 @@ module: lp.code.interfaces.gitjob
712 dbuser: send-branch-mail
713 crontab_group: MAIN
714
715+[IGitRepositoryChangeInformationTypeJobSource]
716+module: lp.code.interfaces.gitjob
717+dbuser: privacy-change-jobs
718+crontab_group: MAIN
719+
720 [IInitializeDistroSeriesJobSource]
721 module: lp.soyuz.interfaces.distributionjob
722 dbuser: initializedistroseries
723diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
724index 57babc5..9d1f53b 100644
725--- a/lib/lp/testing/factory.py
726+++ b/lib/lp/testing/factory.py
727@@ -1815,7 +1815,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
728 reviewer=reviewer, **optional_repository_args)
729 naked_repository = removeSecurityProxy(repository)
730 if information_type is not None:
731- naked_repository.transitionToInformationType(
732+ naked_repository._transitionToInformationType(
733 information_type, registrant, verify_policy=False)
734 return repository
735