Merge ~pappacena/launchpad:git-repo-async-privacy into launchpad: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 Needs Information
Ioana Lasc 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).

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

LGTM

review: Approve
974b22a... by Thiago F. Pappacena on 2020-10-29

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
826cbad... by Thiago F. Pappacena on 2020-11-05

Encapsulating GitRepository.pending_change_information_type

eeeb231... by Thiago F. Pappacena on 2020-11-05

Renaming GitRepo information type change job

2493c1d... by Thiago F. Pappacena on 2020-11-05

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.

Unmerged commits

2493c1d... by Thiago F. Pappacena on 2020-11-05

Changing exception msg when blocking info type transition for git repo

eeeb231... by Thiago F. Pappacena on 2020-11-05

Renaming GitRepo information type change job

826cbad... by Thiago F. Pappacena on 2020-11-05

Encapsulating GitRepository.pending_change_information_type

974b22a... by Thiago F. Pappacena on 2020-10-29

Fixing test

4bd7dcc... by Thiago F. Pappacena on 2020-10-26

Fixing configuration of info type transition job

76e0272... by Thiago F. Pappacena on 2020-10-23

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

caf2673... by Thiago F. Pappacena on 2020-10-23

Adding tests for async GitRepo.transitionToInformationType

3a5e053... by Thiago F. Pappacena on 2020-10-23

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