launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25591
[Merge] ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master.
Commit message:
Doing virtual refs sync when running git repository privacy change background task
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/393074
This branch carries code from https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/390581, but it does a big chunch of refactoring of the code contained there. Mostly, the bulk operations got encapsulated in the GitHostingClient, so we would have a single point of change once Turnip will be able to run the copy/delete refs operations in bulk (instead of doing a bunch of requests).
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 11aba6f..94cb548 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -2082,6 +2082,24 @@ public.webhookjob = SELECT, INSERT
public.xref = SELECT, INSERT, DELETE
type=user
+[privacy-change-jobs]
+groups=script, branchscanner
+public.accessartifact = SELECT, UPDATE, DELETE, INSERT
+public.accessartifactgrant = SELECT, UPDATE, DELETE, INSERT
+public.accesspolicyartifact = SELECT, UPDATE, DELETE, INSERT
+public.accesspolicygrant = SELECT, UPDATE, DELETE
+public.account = SELECT
+public.distribution = SELECT
+public.gitjob = SELECT, UPDATE
+public.gitrepository = SELECT, UPDATE
+public.gitsubscription = SELECT, UPDATE, DELETE
+public.job = SELECT, INSERT, UPDATE
+public.person = SELECT
+public.product = SELECT
+public.sharingjob = SELECT, INSERT, UPDATE
+public.teamparticipation = SELECT
+type=user
+
[sharing-jobs]
groups=script
public.accessartifactgrant = SELECT, UPDATE, DELETE
diff --git a/lib/lp/app/widgets/itemswidgets.py b/lib/lp/app/widgets/itemswidgets.py
index 1dbb59f..a644a96 100644
--- a/lib/lp/app/widgets/itemswidgets.py
+++ b/lib/lp/app/widgets/itemswidgets.py
@@ -189,25 +189,29 @@ class LaunchpadRadioWidgetWithDescription(LaunchpadRadioWidget):
"""Render an item of the list."""
text = html_escape(text)
id = '%s.%s' % (name, index)
+ extra_attr = {"disabled": "disabled"} if self.context.readonly else {}
elem = renderElement(u'input',
value=value,
name=name,
id=id,
cssClass=cssClass,
- type='radio')
+ type='radio',
+ **extra_attr)
return self._renderRow(text, value, id, elem)
def renderSelectedItem(self, index, text, value, name, cssClass):
"""Render a selected item of the list."""
text = html_escape(text)
id = '%s.%s' % (name, index)
+ extra_attr = {"disabled": "disabled"} if self.context.readonly else {}
elem = renderElement(u'input',
value=value,
name=name,
id=id,
cssClass=cssClass,
checked="checked",
- type='radio')
+ type='radio',
+ **extra_attr)
return self._renderRow(text, value, id, elem)
def renderExtraHint(self):
diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
index 2552ffb..029e7ba 100644
--- a/lib/lp/code/browser/gitrepository.py
+++ b/lib/lp/code/browser/gitrepository.py
@@ -483,6 +483,9 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
def warning_message(self):
if self.context.status == GitRepositoryStatus.CREATING:
return "This repository is being created."
+ if (self.context.status ==
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION):
+ return "This repository's information type is being changed."
return None
@property
@@ -573,6 +576,9 @@ class GitRepositoryEditFormView(LaunchpadEditFormView):
@cachedproperty
def schema(self):
info_types = self.getInformationTypesToShow()
+ read_only_info_type = (
+ self.context.status ==
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
class GitRepositoryEditSchema(Interface):
"""Defines the fields for the edit form.
@@ -582,7 +588,8 @@ class GitRepositoryEditFormView(LaunchpadEditFormView):
"""
use_template(IGitRepository, include=["default_branch"])
information_type = copy_field(
- IGitRepository["information_type"], readonly=False,
+ IGitRepository["information_type"],
+ readonly=read_only_info_type,
vocabulary=InformationTypeVocabulary(types=info_types))
name = copy_field(IGitRepository["name"], readonly=False)
owner = copy_field(IGitRepository["owner"], readonly=False)
@@ -785,6 +792,11 @@ class GitRepositoryEditView(CodeEditOwnerMixin, GitRepositoryEditFormView):
self.widgets["target"].hint = (
"This is the default repository for this target, so it "
"cannot be moved to another target.")
+ if (self.context.status ==
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION):
+ self.widgets["information_type"].hint = (
+ "Information type is being changed. The operation needs to "
+ "finish before you can changing it again.")
if self.context.default_branch:
self.widgets['default_branch'].context.required = True
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index f2ec4ec..83c51dc 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -2336,7 +2336,7 @@ class TestLatestProposalsForEachBranchGit(
@staticmethod
def _setBranchInvisible(branch):
- removeSecurityProxy(branch.repository).transitionToInformationType(
+ removeSecurityProxy(branch.repository)._transitionToInformationType(
InformationType.USERDATA, branch.owner, verify_policy=False)
diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index 912965d..8a9af9f 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -60,6 +60,9 @@ from lp.code.enums import (
GitRepositoryType,
)
from lp.code.interfaces.gitcollection import IGitCollection
+from lp.code.interfaces.gitjob import (
+ IGitRepositoryTransitionToInformationTypeJobSource,
+ )
from lp.code.interfaces.gitrepository import IGitRepositorySet
from lp.code.interfaces.revision import IRevisionSet
from lp.code.model.gitjob import GitRefScanJob
@@ -161,6 +164,15 @@ class TestGitRepositoryView(BrowserTestCase):
self.assertTextMatchesExpressionIgnoreWhitespace(
r"""This repository is being created\..*""", text)
+ def test_changing_info_type_warning_message_is_present(self):
+ repository = removeSecurityProxy(self.factory.makeGitRepository())
+ repository.status = (
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+ text = self.getMainText(repository, "+index", user=repository.owner)
+ self.assertTextMatchesExpressionIgnoreWhitespace(
+ r"""This repository's information type is being changed\..*""",
+ text)
+
def test_creating_warning_message_is_not_shown(self):
repository = removeSecurityProxy(self.factory.makeGitRepository())
repository.status = GitRepositoryStatus.AVAILABLE
@@ -1161,7 +1173,50 @@ class TestGitRepositoryEditView(TestCaseWithFactory):
browser.getControl("Change Git Repository").click()
with person_logged_in(person):
self.assertEqual(
- InformationType.USERDATA, repository.information_type)
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repository.status)
+ job_util = getUtility(
+ IGitRepositoryTransitionToInformationTypeJobSource)
+ jobs = list(job_util.iterReady())
+ self.assertEqual(1, len(jobs))
+ job = removeSecurityProxy(jobs[0])
+ self.assertEqual(repository, job.repository)
+ self.assertEqual(InformationType.USERDATA, job.information_type)
+ self.assertEqual(admin, job.user)
+
+ def test_information_type_in_ui_blocked_if_already_changing(self):
+ # The information_type of a repository can't be changed via the UI
+ # if the repository is already pending a info type change.
+ person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(owner=person)
+ removeSecurityProxy(repository).status = (
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+ admin = getUtility(ILaunchpadCelebrities).admin.teamowner
+ browser = self.getUserBrowser(
+ canonical_url(repository) + "/+edit", user=admin)
+ # Make sure the privacy controls are all disabled in the UI.
+ controls = [
+ "Public", "Public Security", "Private Security", "Private",
+ "Proprietary", "Embargoed"]
+ self.assertTrue(
+ all(browser.getControl(i, index=0).disabled for i in controls))
+ expected_msg = (
+ "Information type is being changed. The operation needs to "
+ "finish before you can changing it again.")
+ self.assertIn(expected_msg, extract_text(browser.contents))
+
+ # Trying to change should have no effect in the backend, since the
+ # repository is already changing info type and this field is read-only.
+ browser.getControl("Private", index=1).click()
+ browser.getControl("Change Git Repository").click()
+ with person_logged_in(person):
+ self.assertEqual(
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repository.status)
+ job_util = getUtility(
+ IGitRepositoryTransitionToInformationTypeJobSource)
+ jobs = list(job_util.iterReady())
+ self.assertEqual(0, len(jobs))
def test_edit_view_ajax_render(self):
# An information type change request is processed as expected when
@@ -1184,7 +1239,17 @@ class TestGitRepositoryEditView(TestCaseWithFactory):
result = view.render()
self.assertEqual("", result)
self.assertEqual(
- repository.information_type, InformationType.PUBLICSECURITY)
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repository.status)
+ job_util = getUtility(
+ IGitRepositoryTransitionToInformationTypeJobSource)
+ jobs = list(job_util.iterReady())
+ self.assertEqual(1, len(jobs))
+ job = removeSecurityProxy(jobs[0])
+ self.assertEqual(repository, job.repository)
+ self.assertEqual(
+ InformationType.PUBLICSECURITY, job.information_type)
+ self.assertEqual(person, job.user)
def test_change_default_branch(self):
# An authorised user can change the default branch to one that
diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml
index 898e645..fee38a9 100644
--- a/lib/lp/code/configure.zcml
+++ b/lib/lp/code/configure.zcml
@@ -1111,6 +1111,11 @@
provides="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource">
<allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJobSource" />
</securedutility>
+ <securedutility
+ component="lp.code.model.gitjob.GitRepositoryTransitionToInformationTypeJob"
+ provides="lp.code.interfaces.gitjob.IGitRepositoryTransitionToInformationTypeJobSource">
+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryTransitionToInformationTypeJobSource" />
+ </securedutility>
<class class="lp.code.model.gitjob.GitRefScanJob">
<allow interface="lp.code.interfaces.gitjob.IGitJob" />
<allow interface="lp.code.interfaces.gitjob.IGitRefScanJob" />
@@ -1123,6 +1128,10 @@
<allow interface="lp.code.interfaces.gitjob.IGitJob" />
<allow interface="lp.code.interfaces.gitjob.IGitRepositoryModifiedMailJob" />
</class>
+ <class class="lp.code.model.gitjob.GitRepositoryTransitionToInformationTypeJob">
+ <allow interface="lp.code.interfaces.gitjob.IGitJob" />
+ <allow interface="lp.code.interfaces.gitjob.IGitRepositoryTransitionToInformationTypeJob" />
+ </class>
<lp:help-folder folder="help" name="+help-code" />
diff --git a/lib/lp/code/enums.py b/lib/lp/code/enums.py
index d2a9262..2edf401 100644
--- a/lib/lp/code/enums.py
+++ b/lib/lp/code/enums.py
@@ -167,6 +167,12 @@ class GitRepositoryStatus(DBEnumeratedType):
This repository is available to be used.
""")
+ PENDING_INFORMATION_TYPE_TRANSITION = DBItem(3, """
+ Information type transition pending
+
+ This repository's privacy setting is being changed.
+ """)
+
class GitObjectType(DBEnumeratedType):
"""Git Object Type
diff --git a/lib/lp/code/interfaces/branchmergeproposal.py b/lib/lp/code/interfaces/branchmergeproposal.py
index bf481c9..efe1e4a 100644
--- a/lib/lp/code/interfaces/branchmergeproposal.py
+++ b/lib/lp/code/interfaces/branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""The interface for branch merge proposals."""
@@ -519,6 +519,50 @@ class IBranchMergeProposalView(Interface):
def getLatestDiffUpdateJob():
"""Return the latest IUpdatePreviewDiffJob for this MP."""
+ def getGitRefCopyOperations():
+ """Gets the list of GitHosting copy operations that should be done
+ in order to keep this MP's virtual refs up-to-date.
+
+ Note that, for now, the only possible virtual ref that could
+ possibly be copied is GIT_CREATE_MP_VIRTUAL_REF. So, this method
+ will return at most 1 copy operation. Once new virtual refs will be
+ created, this method should be extended to add more copy operations
+ too.
+
+ :return: A list of RefCopyOperation objects.
+ """
+
+ def getGitRefDeleteOperations(force_delete=False):
+ """Gets the list of git refs that should be deleted in order to keep
+ this MP's virtual refs up-to-date.
+
+ Note that, for now, the only existing virtual ref to be deleted is
+ GIT_CREATE_MP_VIRTUAL_REF. So this method will only ever delete at
+ most 1 virtual ref. Once new virtual refs will be created, this method
+ should be extended to delete them also.
+
+ :param force_delete: True if we should get delete operation
+ regardless of any check. False otherwise.
+ :return: A list of ref names.
+ """
+
+ def syncGitVirtualRefs(force_delete=False):
+ """Requests all copies and deletion of virtual refs to make git code
+ hosting in sync with this MP.
+
+ :param force_delete: Do not try to copy any new virtual ref. Just
+ delete all virtual refs possibly created.
+ """
+
+ def bulkSyncGitVirtualRefs(merge_proposals):
+ """Synchronizes a set of merge proposals' virtual refs.
+
+ :params merge_proposals: The set of merge proposals that should have
+ the virtual refs synced.
+ :return: A tuple with the list of (copy operations, delete operations)
+ executed.
+ """
+
class IBranchMergeProposalEdit(Interface):
diff --git a/lib/lp/code/interfaces/githosting.py b/lib/lp/code/interfaces/githosting.py
index 441e850..7fe0d0e 100644
--- a/lib/lp/code/interfaces/githosting.py
+++ b/lib/lp/code/interfaces/githosting.py
@@ -148,3 +148,11 @@ class IGitHostingClient(Interface):
:param refs: A list of tuples like (repo_path, ref_name) to be deleted.
:param logger: An optional logger.
"""
+
+ def bulkSyncRefs(copy_operations, delete_operations):
+ """Executes all copy operations and delete operations on Turnip
+ side.
+
+ :param copy_operations: The list of RefCopyOperation to run.
+ :param delete_operations: The list of RefDeleteOperation to run.
+ """
diff --git a/lib/lp/code/interfaces/gitjob.py b/lib/lp/code/interfaces/gitjob.py
index 4f31b19..bfe5525 100644
--- a/lib/lp/code/interfaces/gitjob.py
+++ b/lib/lp/code/interfaces/gitjob.py
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""GitJob interfaces."""
@@ -11,6 +11,8 @@ __all__ = [
'IGitRefScanJobSource',
'IGitRepositoryModifiedMailJob',
'IGitRepositoryModifiedMailJobSource',
+ 'IGitRepositoryTransitionToInformationTypeJob',
+ 'IGitRepositoryTransitionToInformationTypeJobSource',
'IReclaimGitRepositorySpaceJob',
'IReclaimGitRepositorySpaceJobSource',
]
@@ -93,3 +95,20 @@ class IGitRepositoryModifiedMailJobSource(IJobSource):
:param repository_delta: An `IGitRepositoryDelta` describing the
changes.
"""
+
+
+class IGitRepositoryTransitionToInformationTypeJob(IRunnableJob):
+ """A Job to change repository's information type."""
+
+
+class IGitRepositoryTransitionToInformationTypeJobSource(IJobSource):
+
+ def create(repository, user, information_type, verify_policy=True):
+ """Create a job to change git repository's information type.
+
+ :param repository: The `IGitRepository` that was modified.
+ :param information_type: The `InformationType` to transition to.
+ :param user: The `IPerson` who is making the change.
+ :param verify_policy: Check if the new information type complies
+ with the `IGitNamespacePolicy`.
+ """
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index 192133a..48e7dd3 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -8,6 +8,8 @@ __metaclass__ = type
__all__ = [
'ContributorGitIdentity',
'GitIdentityMixin',
+ 'GIT_CREATE_MP_VIRTUAL_REF',
+ 'GIT_MP_VIRTUAL_REF_FORMAT',
'GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE',
'git_repository_name_validator',
'IGitRepository',
@@ -105,6 +107,11 @@ valid_git_repository_name_pattern = re.compile(
r"^(?i)[a-z0-9][a-z0-9+\.\-@_]*\Z")
+# Virtual ref where we automatically put a copy of any open merge proposal.
+GIT_MP_VIRTUAL_REF_FORMAT = 'refs/merge/{mp.id}/head'
+GIT_CREATE_MP_VIRTUAL_REF = 'git.mergeproposal_virtualref.enabled'
+
+
def valid_git_repository_name(name):
"""Return True iff the name is valid as a Git repository name.
@@ -595,11 +602,15 @@ class IGitRepositoryView(IHasRecipes):
def updateLandingTargets(paths):
"""Update landing targets (MPs where this repository is the source).
- For each merge proposal, create `UpdatePreviewDiffJob`s.
+ For each merge proposal, create `UpdatePreviewDiffJob`s, and runs
+ the appropriate GitHosting.copyRefs operation to allow having
+ virtual merge refs with the updated branch version.
:param paths: A list of reference paths. Any merge proposals whose
source is this repository and one of these paths will have their
diffs updated.
+ :return: Returns a tuple with the list of background jobs created,
+ and the list of ref copies requested to GitHosting.
"""
def markRecipesStale(paths):
diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py
index 061fb1e..821b15d 100644
--- a/lib/lp/code/model/branchmergeproposal.py
+++ b/lib/lp/code/model/branchmergeproposal.py
@@ -12,6 +12,7 @@ __all__ = [
from collections import defaultdict
from email.utils import make_msgid
+import logging
from operator import attrgetter
import sys
@@ -84,7 +85,12 @@ from lp.code.interfaces.codereviewcomment import ICodeReviewComment
from lp.code.interfaces.codereviewinlinecomment import (
ICodeReviewInlineCommentSet,
)
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitrepository import (
+ GIT_CREATE_MP_VIRTUAL_REF,
+ GIT_MP_VIRTUAL_REF_FORMAT,
+ )
from lp.code.mail.branch import RecipientReason
from lp.code.model.branchrevision import BranchRevision
from lp.code.model.codereviewcomment import CodeReviewComment
@@ -94,6 +100,10 @@ from lp.code.model.diff import (
IncrementalDiff,
PreviewDiff,
)
+from lp.code.model.githosting import (
+ RefCopyOperation,
+ RefDeleteOperation,
+ )
from lp.registry.interfaces.person import (
IPerson,
IPersonSet,
@@ -123,6 +133,7 @@ from lp.services.database.sqlbase import (
quote,
SQLBase,
)
+from lp.services.features import getFeatureFlag
from lp.services.helpers import shortlist
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
@@ -144,6 +155,9 @@ from lp.soyuz.enums import (
)
+logger = logging.getLogger(__name__)
+
+
def is_valid_transition(proposal, from_state, next_state, user=None):
"""Is it valid for this user to move this proposal to to next_state?
@@ -971,6 +985,7 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
branch_merge_proposal=self.id):
job.destroySelf()
self._preview_diffs.remove()
+
self.destroySelf()
def getUnlandedSourceBranchRevisions(self):
@@ -1217,6 +1232,89 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin):
for diff in diffs)
return [diff_dict.get(revisions) for revisions in revision_list]
+ @property
+ def should_have_git_virtual_refs(self):
+ """True if this MP should have virtual refs in the target
+ repository. False otherwise."""
+ # If the source repository is private and it's opening a MP to
+ # another repository, we should not risk disclosing the private code to
+ # everyone with permission to see the target repo:
+ private_source = self.source_git_repository.private
+ same_owner = self.source_git_repository.owner.inTeam(
+ self.target_git_repository.owner)
+ return not private_source or same_owner
+
+ def getGitRefCopyOperations(self):
+ """See `IBranchMergeProposal`"""
+ if (not self.target_git_repository
+ or not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF)):
+ # Not a git MP. Skip.
+ return []
+ if not self.should_have_git_virtual_refs:
+ return []
+ return [RefCopyOperation(
+ self.source_git_repository.getInternalPath(),
+ self.source_git_commit_sha1,
+ self.target_git_repository.getInternalPath(),
+ GIT_MP_VIRTUAL_REF_FORMAT.format(mp=self))]
+
+ def getGitRefDeleteOperations(self, force_delete=False):
+ """See `IBranchMergeProposal`"""
+ if self.source_git_ref is None:
+ # Not a git MP. Skip.
+ return []
+ if not force_delete and self.should_have_git_virtual_refs:
+ return []
+ return [RefDeleteOperation(
+ self.target_git_repository.getInternalPath(),
+ GIT_MP_VIRTUAL_REF_FORMAT.format(mp=self))]
+
+ def syncGitVirtualRefs(self, force_delete=False):
+ """See `IBranchMergeProposal`"""
+ if self.source_git_ref is None:
+ return
+ if not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF):
+ # XXX pappacena 2020-09-15: Do not try to remove virtual refs if
+ # the feature is disabled. It might be disabled due to bug on
+ # the first versions, for example, and we should have a way to
+ # skip this feature entirely.
+ # Once the feature is stable, we should actually *always* delete
+ # the virtual refs, since we don't know if the feature was
+ # enabled or not when the branch was created.
+ return
+ if force_delete:
+ copy_operations = []
+ else:
+ copy_operations = self.getGitRefCopyOperations()
+ delete_operations = self.getGitRefDeleteOperations(force_delete)
+
+ if copy_operations or delete_operations:
+ hosting_client = getUtility(IGitHostingClient)
+ hosting_client.bulkSyncRefs(copy_operations, delete_operations)
+
+ @classmethod
+ def bulkSyncGitVirtualRefs(cls, merge_proposals):
+ """See `IBranchMergeProposal`"""
+ if not getFeatureFlag(GIT_CREATE_MP_VIRTUAL_REF):
+ # XXX pappacena 2020-09-15: Do not try to remove virtual refs if
+ # the feature is disabled. It might be disabled due to bug on
+ # the first versions, for example, and we should have a way to
+ # skip this feature entirely.
+ # Once the feature is stable, we should actually *always* delete
+ # the virtual refs, since we don't know if the feature was
+ # enabled or not when the branch was created.
+ return [], []
+ copy_operations = []
+ delete_operations = []
+ for merge_proposal in merge_proposals:
+ copy_operations += merge_proposal.getGitRefCopyOperations()
+ delete_operations += merge_proposal.getGitRefDeleteOperations()
+
+ if copy_operations or delete_operations:
+ hosting_client = getUtility(IGitHostingClient)
+ hosting_client.bulkSyncRefs(copy_operations, delete_operations)
+ return copy_operations, delete_operations
+
def scheduleDiffUpdates(self, return_jobs=True):
"""See `IBranchMergeProposal`."""
from lp.code.model.branchmergeproposaljob import (
diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py
index 74f4ec0..b2632f6 100644
--- a/lib/lp/code/model/githosting.py
+++ b/lib/lp/code/model/githosting.py
@@ -7,9 +7,11 @@ __metaclass__ = type
__all__ = [
'GitHostingClient',
'RefCopyOperation',
+ 'RefDeleteOperation',
]
import base64
+from collections import defaultdict
import json
import sys
@@ -33,7 +35,6 @@ from lp.code.errors import (
GitRepositoryDeletionFault,
GitRepositoryScanFault,
GitTargetError,
- NoSuchGitReference,
)
from lp.code.interfaces.githosting import IGitHostingClient
from lp.services.config import config
@@ -55,12 +56,21 @@ class RefCopyOperation:
This class is just a helper to define copy operations parameters on
IGitHostingClient.copyRefs method.
"""
- def __init__(self, source_ref, target_repo, target_ref):
+ def __init__(self, source_repo_path, source_ref, target_repo_path,
+ target_ref):
+ self.source_repo_path = source_repo_path
self.source_ref = source_ref
- self.target_repo = target_repo
+ self.target_repo_path = target_repo_path
self.target_ref = target_ref
+class RefDeleteOperation:
+ """A description of a ref delete operation."""
+ def __init__(self, repo_path, ref):
+ self.repo_path = repo_path
+ self.ref = ref
+
+
@implementer(IGitHostingClient)
class GitHostingClient:
"""A client for the internal API provided by the Git hosting system."""
@@ -277,7 +287,7 @@ class GitHostingClient:
json_data = {
"operations": [{
"from": i.source_ref,
- "to": {"repo": i.target_repo, "ref": i.target_ref}
+ "to": {"repo": i.target_repo_path, "ref": i.target_ref}
} for i in operations]
}
try:
@@ -299,6 +309,8 @@ class GitHostingClient:
def deleteRefs(self, refs, logger=None):
"""See `IGitHostingClient`."""
+ # XXX pappacena: This needs to be done in a bulk operation,
+ # instead of several requests.
for path, ref in refs:
try:
if logger is not None:
@@ -309,3 +321,16 @@ class GitHostingClient:
raise GitReferenceDeletionFault(
"Error deleting %s from repo %s: HTTP %s" %
(ref, path, e.response.status_code))
+
+ def bulkSyncRefs(self, copy_operations, delete_operations):
+ """See `IGitHostingClient`."""
+ # XXX pappacena: This needs to be done in a bulk operation on Turnip
+ # side, instead of several requests.
+ operations_per_path = defaultdict(list)
+ for operation in copy_operations:
+ operations_per_path[operation.source_repo_path].append(operation)
+ for repo_path, operations in operations_per_path:
+ self.copyRefs(repo_path, operations)
+
+ self.deleteRefs([(i.repo_path, i.ref) for i in delete_operations])
+
diff --git a/lib/lp/code/model/gitjob.py b/lib/lp/code/model/gitjob.py
index 1eb87da..77135b3 100644
--- a/lib/lp/code/model/gitjob.py
+++ b/lib/lp/code/model/gitjob.py
@@ -12,6 +12,7 @@ __all__ = [
'ReclaimGitRepositorySpaceJob',
]
+from itertools import chain
from lazr.delegates import delegate_to
from lazr.enum import (
@@ -33,10 +34,15 @@ from zope.interface import (
provider,
)
+from lp.app.enums import (
+ InformationType,
+ PRIVATE_INFORMATION_TYPES,
+ )
from lp.app.errors import NotFoundError
from lp.code.enums import (
GitActivityType,
GitPermissionType,
+ GitRepositoryStatus,
)
from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitjob import (
@@ -45,6 +51,8 @@ from lp.code.interfaces.gitjob import (
IGitRefScanJobSource,
IGitRepositoryModifiedMailJob,
IGitRepositoryModifiedMailJobSource,
+ IGitRepositoryTransitionToInformationTypeJob,
+ IGitRepositoryTransitionToInformationTypeJobSource,
IReclaimGitRepositorySpaceJob,
IReclaimGitRepositorySpaceJobSource,
)
@@ -100,6 +108,13 @@ class GitJobType(DBEnumeratedType):
modifications.
""")
+ REPOSITORY_TRANSITION_TO_INFO_TYPE = DBItem(3, """
+ Change repository's information type
+
+ This job runs when a user requests to change privacy settings of a
+ repository.
+ """)
+
@implementer(IGitJob)
class GitJob(StormBase):
@@ -393,3 +408,72 @@ class GitRepositoryModifiedMailJob(GitJobDerived):
def run(self):
"""See `IGitRepositoryModifiedMailJob`."""
self.getMailer().sendAll()
+
+
+@implementer(IGitRepositoryTransitionToInformationTypeJob)
+@provider(IGitRepositoryTransitionToInformationTypeJobSource)
+class GitRepositoryTransitionToInformationTypeJob(GitJobDerived):
+ """A Job to change git repository's information type."""
+
+ class_job_type = GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE
+
+ config = config.IGitRepositoryTransitionToInformationTypeJobSource
+
+ @classmethod
+ def create(cls, repository, information_type, user, verify_policy=True):
+ """See `IGitRepositoryTransitionToInformationTypeJobSource`."""
+ metadata = {
+ "user": user.id,
+ "information_type": information_type.value,
+ "verify_policy": verify_policy,
+ }
+ git_job = GitJob(repository, cls.class_job_type, metadata)
+ job = cls(git_job)
+ job.celeryRunOnCommit()
+ return job
+
+ @property
+ def user(self):
+ return getUtility(IPersonSet).get(self.metadata["user"])
+
+ @property
+ def verify_policy(self):
+ return self.metadata["verify_policy"]
+
+ @property
+ def information_type(self):
+ return InformationType.items[self.metadata["information_type"]]
+
+ def updateVirtualRefs(self):
+ copy_operations = []
+ delete_operations = []
+ merge_proposals = chain(
+ self.repository.landing_targets,
+ self.repository.landing_candidates)
+ for mp in merge_proposals:
+ copy_operations += mp.getGitRefCopyOperations()
+ delete_operations += mp.getGitRefDeleteOperations()
+ hosting_client = getUtility(IGitHostingClient)
+ hosting_client.bulkSyncRefs(copy_operations, delete_operations)
+
+ def run(self):
+ """See `IGitRepositoryTransitionToInformationTypeJob`."""
+ if (self.repository.status !=
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION):
+ raise AttributeError(
+ "The repository %s is not pending information type change." %
+ self.repository)
+ is_private = self.repository.private
+ will_be_private = self.information_type in PRIVATE_INFORMATION_TYPES
+
+ self.repository._transitionToInformationType(
+ self.information_type, self.user, self.verify_policy)
+
+ # When changing privacy type, we need to make sure to not leak
+ # anything though virtual refs, and create new ones that didn't
+ # exist before.
+ if is_private != will_be_private:
+ # XXX pappacena 2020-10-28: We need to implement retry rules here.
+ self.updateVirtualRefs()
+
+ self.repository.status = GitRepositoryStatus.AVAILABLE
diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index f7dc142..e2f788d 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -180,7 +180,7 @@ class GitRefMixin:
def transitionToInformationType(self, information_type, user,
verify_policy=True):
- return self.repository.transitionToInformationType(
+ return self.repository._transitionToInformationType(
information_type, user, verify_policy=verify_policy)
@property
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index fb5e703..731ca3f 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -117,7 +117,10 @@ from lp.code.interfaces.gitcollection import (
IGitCollection,
)
from lp.code.interfaces.githosting import IGitHostingClient
-from lp.code.interfaces.gitjob import IGitRefScanJobSource
+from lp.code.interfaces.gitjob import (
+ IGitRefScanJobSource,
+ IGitRepositoryTransitionToInformationTypeJobSource,
+ )
from lp.code.interfaces.gitlookup import IGitLookup
from lp.code.interfaces.gitnamespace import (
get_git_namespace,
@@ -882,6 +885,24 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
def transitionToInformationType(self, information_type, user,
verify_policy=True):
"""See `IGitRepository`."""
+ if self.status != GitRepositoryStatus.AVAILABLE:
+ raise CannotChangeInformationType(
+ "Cannot change privacy settings while git repository is "
+ "being changed.")
+ self.status = GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION
+ util = getUtility(
+ IGitRepositoryTransitionToInformationTypeJobSource)
+ return util.create(self, information_type, user, verify_policy)
+
+ def _transitionToInformationType(self, information_type, user,
+ verify_policy=True):
+ """Synchronously make the change in this repository's information
+ type.
+
+ External callers should use the async, public version of this
+ method, since it deals with the side effects of changing
+ repository's privacy changes.
+ """
if self.information_type == information_type:
return
if (verify_policy and
@@ -1184,9 +1205,12 @@ class GitRepository(StormBase, WebhookTargetMixin, GitIdentityMixin):
def updateLandingTargets(self, paths):
"""See `IGitRepository`."""
jobs = []
- for merge_proposal in self.getActiveLandingTargets(paths):
+ merge_proposals = self.getActiveLandingTargets(paths)
+ for merge_proposal in merge_proposals:
jobs.extend(merge_proposal.scheduleDiffUpdates())
- return jobs
+ copy_ops, delete_ops = BranchMergeProposal.bulkSyncGitVirtualRefs(
+ merge_proposals)
+ return jobs, copy_ops, delete_ops
def _getRecipes(self, paths=None):
"""Undecorated version of recipes for use by `markRecipesStale`."""
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index 81b94b3..b601ad8 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -67,6 +67,10 @@ from lp.code.interfaces.branchmergeproposal import (
IBranchMergeProposalGetter,
IBranchMergeProposalJobSource,
)
+from lp.code.interfaces.gitrepository import (
+ GIT_CREATE_MP_VIRTUAL_REF,
+ GIT_MP_VIRTUAL_REF_FORMAT,
+ )
from lp.code.model.branchmergeproposal import (
BranchMergeProposal,
BranchMergeProposalGetter,
@@ -97,6 +101,7 @@ from lp.testing import (
ExpectedException,
launchpadlib_for,
login,
+ login_admin,
login_person,
person_logged_in,
TestCaseWithFactory,
@@ -256,6 +261,196 @@ class TestBranchMergeProposalPrivacy(TestCaseWithFactory):
self.assertContentEqual([owner, team], subscriptions)
+class TestGitBranchMergeProposalVirtualRefs(TestCaseWithFactory):
+ """Ensure that BranchMergeProposal creation run the appropriate copy
+ and delete of virtual refs, like ref/merge/<id>/head."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestGitBranchMergeProposalVirtualRefs, self).setUp()
+ self.hosting_fixture = self.useFixture(GitHostingFixture())
+
+ def test_should_have_vrefs_public_repos(self):
+ mp = self.factory.makeBranchMergeProposalForGit()
+ self.assertTrue(mp.should_have_git_virtual_refs)
+
+ def test_should_have_vrefs_private_source(self):
+ login_admin()
+ source_repo = self.factory.makeGitRepository(
+ information_type=InformationType.PRIVATESECURITY)
+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+ self.assertFalse(mp.should_have_git_virtual_refs)
+
+ def test_should_have_vrefs_private_target(self):
+ login_admin()
+ source_repo = self.factory.makeGitRepository()
+ target_repo = self.factory.makeGitRepository(
+ target=source_repo.target,
+ information_type=InformationType.PRIVATESECURITY)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+ self.assertTrue(mp.should_have_git_virtual_refs)
+
+ def test_copy_git_merge_virtual_ref(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ mp = self.factory.makeBranchMergeProposalForGit()
+
+ copy_operations = mp.getGitRefCopyOperations()
+ self.assertEqual(1, len(copy_operations))
+ self.assertThat(copy_operations[0], MatchesStructure(
+ source_repo_path=Equals(
+ mp.source_git_repository.getInternalPath()),
+ source_ref=Equals(mp.source_git_commit_sha1),
+ target_repo_path=Equals(
+ mp.target_git_repository.getInternalPath()),
+ target_ref=Equals("refs/merge/%s/head" % mp.id),
+ ))
+ delete_operations = mp.getGitRefDeleteOperations()
+ self.assertEqual([], delete_operations)
+
+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
+ copy_ops, delete_ops = args
+ self.assertEqual({}, kwargs)
+ self.assertEqual([], delete_ops)
+ self.assertEqual(1, len(copy_ops))
+ self.assertThat(copy_ops[0], MatchesStructure(
+ source_repo_path=Equals(
+ mp.source_git_repository.getInternalPath()),
+ source_ref=Equals(mp.source_git_commit_sha1),
+ target_repo_path=Equals(
+ mp.target_git_repository.getInternalPath()),
+ target_ref=Equals("refs/merge/%s/head" % mp.id),
+ ))
+
+ def test_getGitRefCopyOperations_private_source(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ login_admin()
+ source_repo = self.factory.makeGitRepository(
+ information_type=InformationType.PRIVATESECURITY)
+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+ self.assertEqual([], mp.getGitRefCopyOperations())
+
+ delete_operations = mp.getGitRefDeleteOperations()
+ self.assertEqual(1, len(delete_operations))
+ self.assertThat(delete_operations[0], MatchesStructure(
+ repo_path=Equals(mp.target_git_repository.getInternalPath()),
+ ref=Equals("refs/merge/%s/head" % mp.id),
+ ))
+
+ def test_getGitRefCopyOperations_private_source_same_repo(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ login_admin()
+ repo = self.factory.makeGitRepository(
+ information_type=InformationType.PRIVATESECURITY)
+ [source, target] = self.factory.makeGitRefs(
+ repo, ['refs/heads/bugfix', 'refs/heads/master'])
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+ operations = mp.getGitRefCopyOperations()
+ self.assertEqual(1, len(operations))
+ self.assertThat(operations[0], MatchesStructure(
+ source_repo_path=Equals(
+ mp.source_git_repository.getInternalPath()),
+ source_ref=Equals(mp.source_git_commit_sha1),
+ target_repo_path=Equals(
+ mp.target_git_repository.getInternalPath()),
+ target_ref=Equals(GIT_MP_VIRTUAL_REF_FORMAT.format(mp=mp))
+ ))
+
+ delete_operations = mp.getGitRefDeleteOperations()
+ self.assertEqual([], delete_operations)
+
+ def test_getGitRefCopyOperations_private_source_same_owner(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ login_admin()
+ source_repo = self.factory.makeGitRepository(
+ information_type=InformationType.PRIVATESECURITY)
+ target_repo = self.factory.makeGitRepository(
+ target=source_repo.target, owner=source_repo.owner)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+ operations = mp.getGitRefCopyOperations()
+ self.assertEqual(1, len(operations))
+ self.assertThat(operations[0], MatchesStructure(
+ source_repo_path=Equals(
+ mp.source_git_repository.getInternalPath()),
+ source_ref=Equals(mp.source_git_commit_sha1),
+ target_repo_path=Equals(
+ mp.target_git_repository.getInternalPath()),
+ target_ref=Equals(GIT_MP_VIRTUAL_REF_FORMAT.format(mp=mp))
+ ))
+
+ delete_operations = mp.getGitRefDeleteOperations()
+ self.assertEqual([], delete_operations)
+
+ def test_syncGitVirtualRefs(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ login_admin()
+ login_admin()
+ source_repo = self.factory.makeGitRepository()
+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+
+ # mp.syncGitVirtualRefs should have been triggered by event.
+ # Check lp.code.subscribers.branchmergeproposal.merge_proposal_created.
+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
+ copy_ops, delete_ops = args
+ self.assertEquals({}, kwargs)
+ self.assertEqual([], delete_ops)
+ self.assertEqual(1, len(copy_ops))
+ self.assertThat(copy_ops[0], MatchesStructure(
+ source_repo_path=Equals(
+ mp.source_git_repository.getInternalPath()),
+ source_ref=Equals(mp.source_git_commit_sha1),
+ target_repo_path=Equals(
+ mp.target_git_repository.getInternalPath()),
+ target_ref=Equals("refs/merge/%s/head" % mp.id),
+ ))
+
+ self.assertEqual(0, self.hosting_fixture.deleteRefs.call_count)
+
+ def test_syncGitVirtualRefs_private_source_deletes_ref(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ login_admin()
+ source_repo = self.factory.makeGitRepository(
+ information_type=InformationType.PRIVATESECURITY)
+ target_repo = self.factory.makeGitRepository(target=source_repo.target)
+ [source] = self.factory.makeGitRefs(source_repo)
+ [target] = self.factory.makeGitRefs(target_repo)
+ mp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target)
+
+ # mp.syncGitVirtualRefs should have been triggered by event.
+ # Check lp.code.subscribers.branchmergeproposal.merge_proposal_created.
+ self.assertEqual(1, self.hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
+ copy_ops, delete_ops = args
+ self.assertEqual({}, kwargs)
+ self.assertEqual([], copy_ops)
+ self.assertEqual(1, len(delete_ops))
+ self.assertThat(delete_ops[0], MatchesStructure(
+ repo_path=Equals(target_repo.getInternalPath()),
+ ref=Equals("refs/merge/%s/head" % mp.id)))
+
+
class TestBranchMergeProposalTransitions(TestCaseWithFactory):
"""Test the state transitions of branch merge proposals."""
@@ -1185,6 +1380,10 @@ class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
def test_delete_triggers_webhooks(self):
# When an existing merge proposal is deleted, any relevant webhooks
# are triggered.
+ self.useFixture(FeatureFixture({
+ GIT_CREATE_MP_VIRTUAL_REF: "on",
+ BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG: "on"}))
+ hosting_fixture = self.useFixture(GitHostingFixture())
logger = self.useFixture(FakeLogger())
source = self.makeBranch()
target = self.makeBranch(same_target_as=source)
@@ -1203,10 +1402,25 @@ class TestMergeProposalWebhooks(WithVCSScenarios, TestCaseWithFactory):
expected_redacted_payload = dict(
expected_payload,
old=MatchesDict(self.getExpectedPayload(proposal, redact=True)))
+ hosting_fixture = self.useFixture(GitHostingFixture())
proposal.deleteProposal()
delivery = hook.deliveries.one()
+ self.assertIsNotNone(delivery)
self.assertCorrectDelivery(expected_payload, hook, delivery)
self.assertCorrectLogging(expected_redacted_payload, hook, logger)
+ if not self.git:
+ self.assertEqual(0, hosting_fixture.bulkSyncRefs.call_count)
+ else:
+ self.assertEqual(1, hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = hosting_fixture.bulkSyncRefs.calls[0]
+ self.assertEqual({}, kwargs)
+ copy_ops, delete_ops = args
+ self.assertEqual(0, len(copy_ops))
+ self.assertEqual(1, len(delete_ops))
+ self.assertThat(delete_ops[0], MatchesStructure(
+ repo_path=Equals(target.repository.getInternalPath()),
+ ref=Equals("refs/merge/%s/head" % proposal.id)
+ ))
class TestGetAddress(TestCaseWithFactory):
@@ -1507,6 +1721,26 @@ class TestBranchMergeProposalDeletion(TestCaseWithFactory):
self.assertRaises(
SQLObjectNotFound, BranchMergeProposalJob.get, job_id)
+ def test_deleteProposal_for_git_removes_virtual_ref(self):
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ self.useFixture(GitHostingFixture())
+ proposal = self.factory.makeBranchMergeProposalForGit()
+
+ # Clean up fixture calls.
+ hosting_fixture = self.useFixture(GitHostingFixture())
+ proposal.deleteProposal()
+
+ self.assertEqual(1, hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = hosting_fixture.bulkSyncRefs.calls[0]
+ self.assertEqual({}, kwargs)
+ copy_ops, delete_ops = args
+ self.assertEqual([], copy_ops)
+ self.assertEqual(1, len(delete_ops))
+ self.assertThat(delete_ops[0], MatchesStructure(
+ repo_path=Equals(proposal.target_git_repository.getInternalPath()),
+ ref=Equals('refs/merge/%s/head' % proposal.id)
+ ))
+
class TestBranchMergeProposalBugs(WithVCSScenarios, TestCaseWithFactory):
diff --git a/lib/lp/code/model/tests/test_gitcollection.py b/lib/lp/code/model/tests/test_gitcollection.py
index 447fcb4..372a633 100644
--- a/lib/lp/code/model/tests/test_gitcollection.py
+++ b/lib/lp/code/model/tests/test_gitcollection.py
@@ -599,7 +599,7 @@ class TestBranchMergeProposals(TestCaseWithFactory):
registrant = self.factory.makePerson()
mp1 = self.factory.makeBranchMergeProposalForGit(registrant=registrant)
naked_repository = removeSecurityProxy(mp1.target_git_repository)
- naked_repository.transitionToInformationType(
+ naked_repository._transitionToInformationType(
InformationType.USERDATA, registrant, verify_policy=False)
collection = self.all_repositories.visibleByUser(None)
proposals = collection.getMergeProposals()
diff --git a/lib/lp/code/model/tests/test_githosting.py b/lib/lp/code/model/tests/test_githosting.py
index 23d6063..fad0df3 100644
--- a/lib/lp/code/model/tests/test_githosting.py
+++ b/lib/lp/code/model/tests/test_githosting.py
@@ -42,7 +42,6 @@ from lp.code.errors import (
GitRepositoryDeletionFault,
GitRepositoryScanFault,
GitTargetError,
- NoSuchGitReference,
)
from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.model.githosting import RefCopyOperation
@@ -414,8 +413,8 @@ class TestGitHostingClient(TestCase):
def getCopyRefOperations(self):
return [
- RefCopyOperation("1a2b3c4", "999", "refs/merge/123"),
- RefCopyOperation("9a8b7c6", "666", "refs/merge/989"),
+ RefCopyOperation("1", "1a2b3c4", "999", "refs/merge/123"),
+ RefCopyOperation("1", "9a8b7c6", "666", "refs/merge/989"),
]
def test_copyRefs(self):
diff --git a/lib/lp/code/model/tests/test_gitjob.py b/lib/lp/code/model/tests/test_gitjob.py
index 3f606c0..69cb653 100644
--- a/lib/lp/code/model/tests/test_gitjob.py
+++ b/lib/lp/code/model/tests/test_gitjob.py
@@ -25,13 +25,16 @@ from testtools.matchers import (
MatchesStructure,
)
import transaction
+from zope.component import getUtility
from zope.interface import providedBy
from zope.security.proxy import removeSecurityProxy
+from lp.app.enums import InformationType
from lp.code.adapters.gitrepository import GitRepositoryDelta
from lp.code.enums import (
GitGranteeType,
GitObjectType,
+ GitRepositoryStatus,
)
from lp.code.interfaces.branchmergeproposal import (
BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG,
@@ -39,6 +42,7 @@ from lp.code.interfaces.branchmergeproposal import (
from lp.code.interfaces.gitjob import (
IGitJob,
IGitRefScanJob,
+ IGitRepositoryTransitionToInformationTypeJobSource,
IReclaimGitRepositorySpaceJob,
)
from lp.code.model.gitjob import (
@@ -47,9 +51,11 @@ from lp.code.model.gitjob import (
GitJobDerived,
GitJobType,
GitRefScanJob,
+ GitRepositoryTransitionToInformationTypeJob,
ReclaimGitRepositorySpaceJob,
)
from lp.code.tests.helpers import GitHostingFixture
+from lp.registry.errors import CannotChangeInformationType
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
from lp.services.features.testing import FeatureFixture
@@ -362,6 +368,65 @@ class TestReclaimGitRepositorySpaceJob(TestCaseWithFactory):
self.assertEqual([(path,)], hosting_fixture.delete.extract_args())
+class TestGitRepositoryTransitionInformationType(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+
+ def test_block_multiple_requests_to_change_info_type(self):
+ repo = self.factory.makeGitRepository()
+ repo.transitionToInformationType(
+ InformationType.PRIVATESECURITY, repo.owner)
+ self.assertEqual(
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repo.status)
+ expected_msg = (
+ "Cannot change privacy settings while git repository is "
+ "being changed.")
+ self.assertRaisesRegex(
+ CannotChangeInformationType, expected_msg,
+ repo.transitionToInformationType,
+ InformationType.PROPRIETARY, repo.owner)
+
+ def test_avoid_transitioning_while_creating(self):
+ repo = self.factory.makeGitRepository()
+ removeSecurityProxy(repo).status = GitRepositoryStatus.CREATING
+ expected_msg = (
+ "Cannot change privacy settings while git repository is "
+ "being changed.")
+ self.assertRaisesRegex(
+ CannotChangeInformationType, expected_msg,
+ repo.transitionToInformationType,
+ InformationType.PROPRIETARY, repo.owner)
+
+ def test_run_changes_info_type(self):
+ repo = self.factory.makeGitRepository(
+ information_type=InformationType.PUBLIC)
+ # Change to a private info type and with verify_policy, so we hit as
+ # many database tables as possible.
+ repo.transitionToInformationType(
+ InformationType.PRIVATESECURITY, repo.owner, verify_policy=True)
+ self.assertEqual(
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repo.status)
+
+ job_util = getUtility(
+ IGitRepositoryTransitionToInformationTypeJobSource)
+ jobs = list(job_util.iterReady())
+ self.assertEqual(1, len(jobs))
+ with dbuser(GitRepositoryTransitionToInformationTypeJob.config.dbuser):
+ JobRunner(jobs).runAll()
+
+ self.assertEqual(GitRepositoryStatus.AVAILABLE, repo.status)
+ self.assertEqual(
+ InformationType.PRIVATESECURITY, repo.information_type)
+
+ # After the job finished, another change is possible.
+ repo.transitionToInformationType(InformationType.PUBLIC, repo.owner)
+ self.assertEqual(
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repo.status)
+
+
class TestDescribeRepositoryDelta(TestCaseWithFactory):
"""Tests for `describe_repository_delta`."""
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index b83f9da..2296352 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -97,6 +97,7 @@ from lp.code.interfaces.gitnamespace import (
IGitNamespaceSet,
)
from lp.code.interfaces.gitrepository import (
+ GIT_CREATE_MP_VIRTUAL_REF,
IGitRepository,
IGitRepositorySet,
IGitRepositoryView,
@@ -783,6 +784,7 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
# Make sure that the tests all flush the database changes.
self.addCleanup(Store.of(self.repository).flush)
login_person(self.user)
+ self.hosting_fixture = self.useFixture(GitHostingFixture())
def test_deletable(self):
# A newly created repository can be deleted without any problems.
@@ -809,7 +811,7 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
def test_private_subscription_does_not_disable_deletion(self):
# A private repository that has a subscription can be deleted.
- self.repository.transitionToInformationType(
+ removeSecurityProxy(self.repository)._transitionToInformationType(
InformationType.USERDATA, self.repository.owner,
verify_policy=False)
self.repository.subscribe(
@@ -824,7 +826,6 @@ class TestGitRepositoryDeletion(TestCaseWithFactory):
def test_code_import_does_not_disable_deletion(self):
# A repository that has an attached code import can be deleted.
- self.useFixture(GitHostingFixture())
code_import = self.factory.makeCodeImport(
target_rcs_type=TargetRevisionControlSystems.GIT)
repository = code_import.git_repository
@@ -984,6 +985,7 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
# unsubscribe the repository owner here.
self.repository.unsubscribe(
self.repository.owner, self.repository.owner)
+ self.hosting_fixture = self.useFixture(GitHostingFixture())
def test_plain_repository(self):
# A fresh repository has no deletion requirements.
@@ -1115,7 +1117,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
def test_code_import_requirements(self):
# Code imports are not included explicitly in deletion requirements.
- self.useFixture(GitHostingFixture())
code_import = self.factory.makeCodeImport(
target_rcs_type=TargetRevisionControlSystems.GIT)
# Remove the implicit repository subscription first.
@@ -1126,7 +1127,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
def test_code_import_deletion(self):
# break_references allows deleting a code import repository.
- self.useFixture(GitHostingFixture())
code_import = self.factory.makeCodeImport(
target_rcs_type=TargetRevisionControlSystems.GIT)
code_import_id = code_import.id
@@ -1182,7 +1182,6 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory):
def test_DeleteCodeImport(self):
# DeleteCodeImport.__call__ must delete the CodeImport.
- self.useFixture(GitHostingFixture())
code_import = self.factory.makeCodeImport(
target_rcs_type=TargetRevisionControlSystems.GIT)
code_import_id = code_import.id
@@ -1521,6 +1520,10 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
+ def setUp(self):
+ super(TestGitRepositoryRefs, self).setUp()
+ self.hosting_fixture = self.useFixture(GitHostingFixture())
+
def test__convertRefInfo(self):
# _convertRefInfo converts a valid info dictionary.
sha1 = six.ensure_text(hashlib.sha1("").hexdigest())
@@ -1791,18 +1794,17 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
# planRefChanges excludes some ref prefixes by default, and can be
# configured otherwise.
repository = self.factory.makeGitRepository()
- hosting_fixture = self.useFixture(GitHostingFixture())
repository.planRefChanges("dummy")
self.assertEqual(
[{"exclude_prefixes": ["refs/changes/"]}],
- hosting_fixture.getRefs.extract_kwargs())
- hosting_fixture.getRefs.calls = []
+ self.hosting_fixture.getRefs.extract_kwargs())
+ self.hosting_fixture.getRefs.calls = []
self.pushConfig(
"codehosting", git_exclude_ref_prefixes="refs/changes/ refs/pull/")
repository.planRefChanges("dummy")
self.assertEqual(
[{"exclude_prefixes": ["refs/changes/", "refs/pull/"]}],
- hosting_fixture.getRefs.extract_kwargs())
+ self.hosting_fixture.getRefs.extract_kwargs())
def test_fetchRefCommits(self):
# fetchRefCommits fetches detailed tip commit metadata for the
@@ -1875,9 +1877,8 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
def test_fetchRefCommits_empty(self):
# If given an empty refs dictionary, fetchRefCommits returns early
# without contacting the hosting service.
- hosting_fixture = self.useFixture(GitHostingFixture())
GitRepository.fetchRefCommits("dummy", {})
- self.assertEqual([], hosting_fixture.getCommits.calls)
+ self.assertEqual([], self.hosting_fixture.getCommits.calls)
def test_synchroniseRefs(self):
# synchroniseRefs copes with synchronising a repository where some
@@ -1920,7 +1921,6 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
self.assertThat(repository.refs, MatchesSetwise(*matchers))
def test_set_default_branch(self):
- hosting_fixture = self.useFixture(GitHostingFixture())
repository = self.factory.makeGitRepository()
self.factory.makeGitRefs(
repository=repository,
@@ -1931,22 +1931,20 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
self.assertEqual(
[((repository.getInternalPath(),),
{"default_branch": "refs/heads/new"})],
- hosting_fixture.setProperties.calls)
+ self.hosting_fixture.setProperties.calls)
self.assertEqual("refs/heads/new", repository.default_branch)
def test_set_default_branch_unchanged(self):
- hosting_fixture = self.useFixture(GitHostingFixture())
repository = self.factory.makeGitRepository()
self.factory.makeGitRefs(
repository=repository, paths=["refs/heads/master"])
removeSecurityProxy(repository)._default_branch = "refs/heads/master"
with person_logged_in(repository.owner):
repository.default_branch = "master"
- self.assertEqual([], hosting_fixture.setProperties.calls)
+ self.assertEqual([], self.hosting_fixture.setProperties.calls)
self.assertEqual("refs/heads/master", repository.default_branch)
def test_set_default_branch_imported(self):
- hosting_fixture = self.useFixture(GitHostingFixture())
repository = self.factory.makeGitRepository(
repository_type=GitRepositoryType.IMPORTED)
self.factory.makeGitRefs(
@@ -1959,7 +1957,7 @@ class TestGitRepositoryRefs(TestCaseWithFactory):
"Cannot modify non-hosted Git repository %s." %
repository.display_name,
setattr, repository, "default_branch", "new")
- self.assertEqual([], hosting_fixture.setProperties.calls)
+ self.assertEqual([], self.hosting_fixture.setProperties.calls)
self.assertEqual("refs/heads/master", repository.default_branch)
def test_exception_unset_default_branch(self):
@@ -2055,7 +2053,8 @@ class TestGitRepositoryModerate(TestCaseWithFactory):
repository.transitionToInformationType(
InformationType.PRIVATESECURITY, project.owner)
self.assertEqual(
- InformationType.PRIVATESECURITY, repository.information_type)
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repository.status)
def test_attribute_smoketest(self):
# Users with launchpad.Moderate can set attributes.
@@ -2580,18 +2579,66 @@ class TestGitRepositoryUpdateLandingTargets(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_schedules_diff_updates(self):
+ def setUp(self):
+ super(TestGitRepositoryUpdateLandingTargets, self).setUp()
+ self.hosting_fixture = self.useFixture(GitHostingFixture())
+
+ def assertSchedulesDiffUpdate(self, with_mp_virtual_ref):
"""Create jobs for all merge proposals."""
bmp1 = self.factory.makeBranchMergeProposalForGit()
bmp2 = self.factory.makeBranchMergeProposalForGit(
source_ref=bmp1.source_git_ref)
- jobs = bmp1.source_git_repository.updateLandingTargets(
- [bmp1.source_git_path])
+
+ # Only enable this virtual ref here, since
+ # self.factory.makeBranchMergeProposalForGit also tries to create
+ # the virtual refs.
+ if with_mp_virtual_ref:
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ else:
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: ""}))
+ jobs, ref_copies, ref_deletes = (
+ bmp1.source_git_repository.updateLandingTargets(
+ [bmp1.source_git_path]))
self.assertEqual(2, len(jobs))
bmps_to_update = [
removeSecurityProxy(job).branch_merge_proposal for job in jobs]
self.assertContentEqual([bmp1, bmp2], bmps_to_update)
+ if not with_mp_virtual_ref:
+ self.assertEqual(
+ 0, self.hosting_fixture.bulkSyncRefs.call_count)
+ else:
+ self.assertEqual(
+ 1, self.hosting_fixture.bulkSyncRefs.call_count)
+ args, kwargs = self.hosting_fixture.bulkSyncRefs.calls[0]
+ self.assertEqual({}, kwargs)
+ self.assertEqual(2, len(args))
+ copy_ops, delete_ops = args
+ self.assertEqual(2, len(copy_ops))
+ self.assertEqual(0, len(delete_ops))
+ self.assertThat(copy_ops[0], MatchesStructure(
+ source_repo_path=Equals(
+ bmp1.source_git_repository.getInternalPath()),
+ source_ref=Equals(bmp1.source_git_commit_sha1),
+ target_repo_path=Equals(
+ bmp1.target_git_repository.getInternalPath()),
+ target_ref=Equals("refs/merge/%s/head" % bmp1.id),
+ ))
+ self.assertThat(copy_ops[1], MatchesStructure(
+ source_repo_path=Equals(
+ bmp2.source_git_repository.getInternalPath()),
+ source_ref=Equals(bmp2.source_git_commit_sha1),
+ target_repo_path=Equals(
+ bmp2.target_git_repository.getInternalPath()),
+ target_ref=Equals("refs/merge/%s/head" % bmp2.id),
+ ))
+
+ def test_schedules_diff_updates_with_mp_virtual_ref(self):
+ self.assertSchedulesDiffUpdate(with_mp_virtual_ref=True)
+
+ def test_schedules_diff_updates_without_mp_virtual_ref(self):
+ self.assertSchedulesDiffUpdate(with_mp_virtual_ref=False)
+
def test_ignores_final(self):
"""Diffs for proposals in final states aren't updated."""
[source_ref] = self.factory.makeGitRefs()
@@ -2603,8 +2650,17 @@ class TestGitRepositoryUpdateLandingTargets(TestCaseWithFactory):
for bmp in source_ref.landing_targets:
if bmp.queue_status not in FINAL_STATES:
removeSecurityProxy(bmp).deleteProposal()
- jobs = source_ref.repository.updateLandingTargets([source_ref.path])
+
+ # Enable the feature here, since factory.makeBranchMergeProposalForGit
+ # would also trigger the copy refs call.
+ self.useFixture(FeatureFixture({GIT_CREATE_MP_VIRTUAL_REF: "on"}))
+ jobs, ref_copies, ref_deletes = (
+ source_ref.repository.updateLandingTargets(
+ [source_ref.path]))
self.assertEqual(0, len(jobs))
+ self.assertEqual(0, len(ref_copies))
+ self.assertEqual(0, len(ref_deletes))
+ self.assertEqual(0, self.hosting_fixture.copyRefs.call_count)
class TestGitRepositoryMarkRecipesStale(TestCaseWithFactory):
@@ -3454,7 +3510,7 @@ class TestGitRepositorySet(TestCaseWithFactory):
]
for repository, modified_date in zip(repositories, modified_dates):
removeSecurityProxy(repository).date_last_modified = modified_date
- removeSecurityProxy(repositories[0]).transitionToInformationType(
+ removeSecurityProxy(repositories[0])._transitionToInformationType(
InformationType.PRIVATESECURITY, repositories[0].registrant)
self.assertEqual(
[repositories[3], repositories[4], repositories[1],
@@ -3967,7 +4023,8 @@ class TestGitRepositoryWebservice(TestCaseWithFactory):
self.assertEqual(209, response.status)
with person_logged_in(ANONYMOUS):
self.assertEqual(
- InformationType.PUBLICSECURITY, repository_db.information_type)
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION,
+ repository_db.status)
def test_set_information_type_other_person(self):
# An unrelated user cannot change the information type.
diff --git a/lib/lp/code/subscribers/branchmergeproposal.py b/lib/lp/code/subscribers/branchmergeproposal.py
index a214c8f..a34ca2a 100644
--- a/lib/lp/code/subscribers/branchmergeproposal.py
+++ b/lib/lp/code/subscribers/branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Event subscribers for branch merge proposals."""
@@ -63,9 +63,13 @@ def _trigger_webhook(merge_proposal, payload):
def merge_proposal_created(merge_proposal, event):
"""A new merge proposal has been created.
- Create a job to update the diff for the merge proposal; trigger webhooks.
+ Create a job to update the diff for the merge proposal; trigger webhooks
+ and copy virtual refs as needed.
"""
getUtility(IUpdatePreviewDiffJobSource).create(merge_proposal)
+
+ merge_proposal.syncGitVirtualRefs()
+
if getFeatureFlag(BRANCH_MERGE_PROPOSAL_WEBHOOKS_FEATURE_FLAG):
payload = {
"action": "created",
@@ -149,3 +153,5 @@ def merge_proposal_deleted(merge_proposal, event):
"old": _compose_merge_proposal_webhook_payload(merge_proposal),
}
_trigger_webhook(merge_proposal, payload)
+
+ merge_proposal.syncGitVirtualRefs(force_delete=True)
diff --git a/lib/lp/code/tests/helpers.py b/lib/lp/code/tests/helpers.py
index 4ef843d..3704b06 100644
--- a/lib/lp/code/tests/helpers.py
+++ b/lib/lp/code/tests/helpers.py
@@ -369,6 +369,9 @@ class GitHostingFixture(fixtures.Fixture):
self.getBlob = FakeMethod(result=blob)
self.delete = FakeMethod()
self.disable_memcache = disable_memcache
+ self.copyRefs = FakeMethod()
+ self.deleteRefs = FakeMethod()
+ self.bulkSyncRefs = FakeMethod()
def _setUp(self):
self.useFixture(ZopeUtilityFixture(self, IGitHostingClient))
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index dcbcee6..e884cd2 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -899,7 +899,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
for _ in range(2)]
private_repository = code_imports[0].git_repository
removeSecurityProxy(
- private_repository).transitionToInformationType(
+ private_repository)._transitionToInformationType(
InformationType.PRIVATESECURITY, private_repository.owner)
with celebrity_logged_in("vcs_imports"):
jobs = [
@@ -1077,7 +1077,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
for _ in range(2)]
private_repository = code_imports[0].git_repository
removeSecurityProxy(
- private_repository).transitionToInformationType(
+ private_repository)._transitionToInformationType(
InformationType.PRIVATESECURITY, private_repository.owner)
with celebrity_logged_in("vcs_imports"):
jobs = [
@@ -1687,7 +1687,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
target_rcs_type=TargetRevisionControlSystems.GIT)
for _ in range(2)]
private_repository = code_imports[0].git_repository
- removeSecurityProxy(private_repository).transitionToInformationType(
+ removeSecurityProxy(private_repository)._transitionToInformationType(
InformationType.PRIVATESECURITY, private_repository.owner)
with celebrity_logged_in("vcs_imports"):
jobs = [
@@ -2012,7 +2012,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
target_rcs_type=TargetRevisionControlSystems.GIT)
for _ in range(2)]
private_repository = code_imports[0].git_repository
- removeSecurityProxy(private_repository).transitionToInformationType(
+ removeSecurityProxy(private_repository)._transitionToInformationType(
InformationType.PRIVATESECURITY, private_repository.owner)
with celebrity_logged_in("vcs_imports"):
jobs = [
@@ -2268,7 +2268,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
target_rcs_type=TargetRevisionControlSystems.GIT)
for _ in range(2)]
private_repository = code_imports[0].git_repository
- removeSecurityProxy(private_repository).transitionToInformationType(
+ removeSecurityProxy(private_repository)._transitionToInformationType(
InformationType.PRIVATESECURITY, private_repository.owner)
with celebrity_logged_in("vcs_imports"):
jobs = [
diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index 6e0feac..ea248a4 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -38,6 +38,7 @@ from storm.expr import (
Cast,
In,
Join,
+ LeftJoin,
Max,
Min,
Or,
@@ -68,6 +69,10 @@ from lp.code.model.diff import (
Diff,
PreviewDiff,
)
+from lp.code.model.gitjob import (
+ GitJob,
+ GitJobType,
+ )
from lp.code.model.gitrepository import GitRepository
from lp.code.model.revision import (
RevisionAuthor,
@@ -1554,6 +1559,54 @@ class GitRepositoryPruner(TunableLoop):
transaction.commit()
+class GitRepositoryBrokenInfoTypeTransition(TunableLoop):
+ """Put back to "AVAILABLE" repositories that are pending information
+ type changes, but we don't have any git job that will actually do that
+ in the upcoming future.
+ """
+
+ maximum_chunk_size = 500
+
+ def __init__(self, log, abort_time=None):
+ super(GitRepositoryBrokenInfoTypeTransition, self).__init__(
+ log, abort_time)
+ self.store = IMasterStore(GitRepository)
+
+ def findRepositories(self):
+ pending_change = (
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+ job_type = GitJobType.REPOSITORY_TRANSITION_TO_INFO_TYPE
+ job_pending_statuses = (JobStatus.WAITING, JobStatus.RUNNING)
+ # Get git repositories left-joining with
+ # REPOSITORY_TRANSITION_TO_INFO_TYPE GitJobs waiting to be run (or
+ # already running).
+ join = [
+ GitRepository,
+ LeftJoin(
+ GitJob,
+ And(GitJob.repository_id == GitRepository.id,
+ GitJob.job_type == job_type)),
+ LeftJoin(
+ Job,
+ And(GitJob.job_id == Job.id,
+ Job._status.is_in(job_pending_statuses)))]
+ # We get only the repositories pending change without associated job.
+ result_set = self.store.using(*join).find(
+ GitRepository,
+ GitRepository.status == pending_change,
+ Job._status == None)
+ return result_set.order_by(GitRepository.date_created)
+
+ def isDone(self):
+ return self.findRepositories().is_empty()
+
+ def __call__(self, chunk_size):
+ repositories = self.findRepositories()[:chunk_size]
+ for repository in repositories:
+ repository.status = GitRepositoryStatus.AVAILABLE
+ transaction.commit()
+
+
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1806,6 +1859,7 @@ class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
BugHeatUpdater,
DuplicateSessionPruner,
GitRepositoryPruner,
+ GitRepositoryBrokenInfoTypeTransition,
RevisionCachePruner,
UnusedSessionPruner,
]
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index 2fbc80d..ae3b0f1 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -1126,6 +1126,66 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
{old_available, recent_available, recent_creating},
set(remaining_repos))
+ def test_GitRepositoryBrokenInfoTypeTransition_changes_status(self):
+ self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
+ # Shortcuts.
+ available = GitRepositoryStatus.AVAILABLE
+ pending_transition = (
+ GitRepositoryStatus.PENDING_INFORMATION_TYPE_TRANSITION)
+
+ switch_dbuser('testadmin')
+ store = IMasterStore(GitRepository)
+ created_repos = [self.factory.makeGitRepository() for i in range(5)]
+
+ pending_without_job_repos = []
+ for i in range(2):
+ repo = self.factory.makeGitRepository()
+ removeSecurityProxy(repo)._status = pending_transition
+
+ pending_with_failed_jobs = []
+ for i in range(3):
+ repo = self.factory.makeGitRepository()
+ # For some repos, create the job but force them to fail
+ job = repo.transitionToInformationType(
+ InformationType.PRIVATESECURITY, repo.owner)
+ job.start()
+ job.fail()
+ pending_with_failed_jobs.append(repo)
+
+ pending_with_started_job_repos = []
+ for i in range(2):
+ repo = self.factory.makeGitRepository()
+ job = repo.transitionToInformationType(
+ InformationType.PRIVATESECURITY, repo.owner)
+ job.start()
+ pending_with_started_job_repos.append(repo)
+
+ pending_with_waiting_jobs = []
+ for i in range(3):
+ repo = self.factory.makeGitRepository()
+ repo.transitionToInformationType(
+ InformationType.PRIVATESECURITY, repo.owner)
+ pending_with_started_job_repos.append(repo)
+
+ self.assertEqual(15, store.find(GitRepository).count())
+
+ self.runHourly(maximum_chunk_size=2)
+
+ switch_dbuser('testadmin')
+ self.assertEqual(15, store.find(GitRepository).count())
+ self.assertTrue(
+ all(i.status == available for i in created_repos))
+ self.assertTrue(
+ all(i.status == available for i in pending_without_job_repos))
+ self.assertTrue(
+ all(i.status == available for i in pending_with_failed_jobs))
+ self.assertTrue(
+ all(i.status == pending_transition
+ for i in pending_with_started_job_repos))
+ self.assertTrue(
+ all(i.status == pending_transition
+ for i in pending_with_waiting_jobs))
+
def test_WebhookJobPruner(self):
# Garbo should remove jobs completed over 30 days ago.
switch_dbuser('testadmin')
diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
index aaa9d30..1a51d27 100644
--- a/lib/lp/services/config/schema-lazr.conf
+++ b/lib/lp/services/config/schema-lazr.conf
@@ -1768,6 +1768,7 @@ job_sources:
ICommercialExpiredJobSource,
IExpiringMembershipNotificationJobSource,
IGitRepositoryModifiedMailJobSource,
+ IGitRepositoryTransitionToInformationTypeJobSource,
IMembershipNotificationJobSource,
IOCIRecipeRequestBuildsJobSource,
IOCIRegistryUploadJobSource,
@@ -1829,6 +1830,11 @@ module: lp.code.interfaces.gitjob
dbuser: send-branch-mail
crontab_group: MAIN
+[IGitRepositoryTransitionToInformationTypeJobSource]
+module: lp.code.interfaces.gitjob
+dbuser: privacy-change-jobs
+crontab_group: MAIN
+
[IInitializeDistroSeriesJobSource]
module: lp.soyuz.interfaces.distributionjob
dbuser: initializedistroseries
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 57babc5..9d1f53b 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -1815,7 +1815,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
reviewer=reviewer, **optional_repository_args)
naked_repository = removeSecurityProxy(repository)
if information_type is not None:
- naked_repository.transitionToInformationType(
+ naked_repository._transitionToInformationType(
information_type, registrant, verify_policy=False)
return repository
References
