launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26689
[Merge] ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master with ~pappacena/launchpad:ocirecipe-filter-private as a prerequisite.
Commit message:
Consider OCIRecupeSubscription on IRemoveArtifactSubscriptionsJobSource
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399889
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 5e5e000..e149a3c 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -2110,6 +2110,9 @@ public.gitrepository = SELECT
public.gitsubscription = SELECT, UPDATE, DELETE
public.emailaddress = SELECT
public.job = SELECT, INSERT, UPDATE
+public.ociproject = SELECT
+public.ocirecipe = SELECT
+public.ocirecipesubscription = SELECT, UPDATE, DELETE
public.person = SELECT
public.product = SELECT
public.sharingjob = SELECT, INSERT, UPDATE
diff --git a/lib/lp/registry/model/sharingjob.py b/lib/lp/registry/model/sharingjob.py
index 81e6a9e..b123547 100644
--- a/lib/lp/registry/model/sharingjob.py
+++ b/lib/lp/registry/model/sharingjob.py
@@ -69,6 +69,12 @@ from lp.code.model.gitrepository import (
GitRepository,
)
from lp.code.model.gitsubscription import GitSubscription
+from lp.oci.interfaces.ocirecipe import IOCIRecipe
+from lp.oci.model.ocirecipe import (
+ get_ocirecipe_privacy_filter,
+ OCIRecipe,
+ )
+from lp.oci.model.ocirecipesubscription import OCIRecipeSubscription
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.sharingjob import (
@@ -78,6 +84,7 @@ from lp.registry.interfaces.sharingjob import (
ISharingJobSource,
)
from lp.registry.model.distribution import Distribution
+from lp.registry.model.ociproject import OCIProject
from lp.registry.model.person import Person
from lp.registry.model.product import Product
from lp.registry.model.teammembership import TeamParticipation
@@ -271,6 +278,7 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
gitrepository_ids = []
snap_ids = []
specification_ids = []
+ ocirecipe_ids = []
if artifacts:
for artifact in artifacts:
if IBug.providedBy(artifact):
@@ -283,6 +291,8 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
snap_ids.append(artifact.id)
elif ISpecification.providedBy(artifact):
specification_ids.append(artifact.id)
+ elif IOCIRecipe.providedBy(artifact):
+ ocirecipe_ids.append(artifact.id)
else:
raise ValueError(
'Unsupported artifact: %r' % artifact)
@@ -295,6 +305,7 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
'gitrepository_ids': gitrepository_ids,
'snap_ids': snap_ids,
'specification_ids': specification_ids,
+ 'ocirecipe_ids': ocirecipe_ids,
'information_types': information_types,
'requestor.id': requestor.id
}
@@ -338,6 +349,10 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
return self.metadata.get('specification_ids', [])
@property
+ def ocirecipe_ids(self):
+ return self.metadata.get('ocirecipe_ids', [])
+
+ @property
def information_types(self):
if not 'information_types' in self.metadata:
return []
@@ -365,6 +380,7 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
'gitrepository_ids': self.gitrepository_ids,
'snap_ids': self.snap_ids,
'specification_ids': self.specification_ids,
+ 'ocirecipe_ids': self.ocirecipe_ids,
'pillar': getattr(self.pillar, 'name', None),
'grantee': getattr(self.grantee, 'name', None)
}
@@ -382,6 +398,7 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
gitrepository_filters = []
snap_filters = []
specification_filters = []
+ ocirecipe_filters = []
if self.branch_ids:
branch_filters.append(Branch.id.is_in(self.branch_ids))
@@ -393,6 +410,8 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
if self.specification_ids:
specification_filters.append(Specification.id.is_in(
self.specification_ids))
+ if self.ocirecipe_ids:
+ ocirecipe_filters.append(OCIRecipe.id.is_in(self.ocirecipe_ids))
if self.bug_ids:
bug_filters.append(BugTaskFlat.bug_id.is_in(self.bug_ids))
else:
@@ -410,6 +429,8 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
specification_filters.append(
Specification.information_type.is_in(
self.information_types))
+ ocirecipe_filters.append(OCIRecipe._information_type.is_in(
+ self.information_types))
if self.product:
bug_filters.append(
BugTaskFlat.product == self.product)
@@ -418,6 +439,11 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
GitRepository.project == self.product)
specification_filters.append(
Specification.product == self.product)
+ ocirecipe_filters.append(
+ In(OCIRecipe.id,
+ Select(OCIRecipe.id,
+ And(OCIRecipe.oci_project_id == OCIProject.id,
+ OCIProject.project == self.product))))
if self.distro:
bug_filters.append(
BugTaskFlat.distribution == self.distro)
@@ -426,6 +452,11 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
GitRepository.distribution == self.distro)
specification_filters.append(
Specification.distribution == self.distro)
+ ocirecipe_filters.append(
+ In(OCIRecipe.id,
+ Select(OCIRecipe.id,
+ And(OCIRecipe.oci_project_id == OCIProject.id,
+ OCIProject.distribution == self.distro))))
if self.grantee:
bug_filters.append(
@@ -450,9 +481,14 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
where=TeamParticipation.team == self.grantee)))
specification_filters.append(
In(SpecificationSubscription.person_id,
- Select(
- TeamParticipation.personID,
- where=TeamParticipation.team == self.grantee)))
+ Select(
+ TeamParticipation.personID,
+ where=TeamParticipation.team == self.grantee)))
+ ocirecipe_filters.append(
+ In(OCIRecipeSubscription.person_id,
+ Select(
+ TeamParticipation.personID,
+ where=TeamParticipation.team == self.grantee)))
if bug_filters:
bug_filters.append(Not(
@@ -517,3 +553,16 @@ class RemoveArtifactSubscriptionsJob(SharingJobDerived):
for sub in specifications_subscriptions:
sub.specification.unsubscribe(
sub.person, self.requestor, ignore_permissions=True)
+ if ocirecipe_filters:
+ ocirecipe_filters.append(
+ Not(get_ocirecipe_privacy_filter(
+ OCIRecipeSubscription.person_id)))
+ ocirecipe_subscriptions = IStore(OCIRecipeSubscription).using(
+ OCIRecipeSubscription,
+ Join(OCIRecipe,
+ OCIRecipe.id == OCIRecipeSubscription.recipe_id)
+ ).find(OCIRecipeSubscription, *ocirecipe_filters).config(
+ distinct=True)
+ for sub in ocirecipe_subscriptions:
+ sub.recipe.unsubscribe(
+ sub.person, self.requestor, ignore_permissions=True)
diff --git a/lib/lp/registry/services/sharingservice.py b/lib/lp/registry/services/sharingservice.py
index fad89b8..4927dc4 100644
--- a/lib/lp/registry/services/sharingservice.py
+++ b/lib/lp/registry/services/sharingservice.py
@@ -39,10 +39,7 @@ from lp.bugs.interfaces.bugtask import IBugTaskSet
from lp.bugs.interfaces.bugtasksearch import BugTaskSearchParams
from lp.code.interfaces.branchcollection import IAllBranches
from lp.code.interfaces.gitcollection import IAllGitRepositories
-from lp.oci.interfaces.ocirecipe import (
- IOCIRecipe,
- IOCIRecipeSet,
- )
+from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
from lp.oci.model.ocirecipe import OCIRecipe
from lp.registry.enums import (
BranchSharingPolicy,
@@ -866,12 +863,6 @@ class SharingService:
if not artifacts:
return
- # XXX: Pappacena 2021-03-09: OCI recipes should not trigger this job,
- # since we do not have a "OCIRecipeSubscription" yet.
- artifacts = [i for i in artifacts if not IOCIRecipe.providedBy(i)]
- if not artifacts:
- return
-
# Create a job to remove subscriptions for artifacts the grantee can no
# longer see.
return getUtility(IRemoveArtifactSubscriptionsJobSource).create(
diff --git a/lib/lp/registry/tests/test_sharingjob.py b/lib/lp/registry/tests/test_sharingjob.py
index 73ad71f..65f382b 100644
--- a/lib/lp/registry/tests/test_sharingjob.py
+++ b/lib/lp/registry/tests/test_sharingjob.py
@@ -16,6 +16,10 @@ from lp.code.enums import (
BranchSubscriptionNotificationLevel,
CodeReviewNotificationLevel,
)
+from lp.oci.interfaces.ocirecipe import (
+ OCI_RECIPE_ALLOW_CREATE,
+ OCI_RECIPE_PRIVATE_FEATURE_FLAG,
+ )
from lp.registry.enums import SpecificationSharingPolicy
from lp.registry.interfaces.accesspolicy import (
IAccessArtifactGrantSource,
@@ -148,6 +152,21 @@ class SharingJobDerivedTestCase(TestCaseWithFactory):
'for requestor=%s, specification_ids=[%d]>'
% (requestor.name, specification.id), repr(job))
+ def test_repr_ocirecipe(self):
+ features = {
+ OCI_RECIPE_ALLOW_CREATE: 'on',
+ OCI_RECIPE_PRIVATE_FEATURE_FLAG: 'on'
+ }
+ self.useFixture(FeatureFixture(features))
+ requestor = self.factory.makePerson()
+ recipe = self.factory.makeOCIRecipe()
+ job = getUtility(IRemoveArtifactSubscriptionsJobSource).create(
+ requestor, artifacts=[recipe])
+ self.assertEqual(
+ '<REMOVE_ARTIFACT_SUBSCRIPTIONS job reconciling subscriptions '
+ 'for ocirecipe_ids=[%d], requestor=%s>'
+ % (recipe.id, requestor.name), repr(job))
+
def test_create_success(self):
# Create an instance of SharingJobDerived that delegates to SharingJob.
self.assertIs(True, ISharingJobSource.providedBy(SharingJobDerived))
@@ -254,6 +273,8 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
def setUp(self):
features = {
'jobs.celery.enabled_classes': 'RemoveArtifactSubscriptionsJob',
+ OCI_RECIPE_ALLOW_CREATE: 'on',
+ OCI_RECIPE_PRIVATE_FEATURE_FLAG: 'on'
}
features.update(SNAP_TESTING_FLAGS)
self.useFixture(FeatureFixture(features))
@@ -334,6 +355,10 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
specification = self.factory.makeSpecification(
owner=owner, product=product,
information_type=InformationType.PROPRIETARY)
+ ocirecipe = self.factory.makeOCIRecipe(
+ owner=owner, registrant=owner,
+ oci_project=self.factory.makeOCIProject(pillar=product),
+ information_type=InformationType.USERDATA)
# The artifact grantees will not lose access when the job is run.
artifact_indirect_grantee = self.factory.makePerson()
@@ -349,6 +374,7 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
BranchSubscriptionNotificationLevel.NOEMAIL, None,
CodeReviewNotificationLevel.NOEMAIL, owner)
snap.subscribe(artifact_indirect_grantee, owner)
+ ocirecipe.subscribe(artifact_indirect_grantee, owner)
# Subscribing somebody to a specification does not automatically
# create an artifact grant.
spec_artifact = self.factory.makeAccessArtifact(specification)
@@ -358,9 +384,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
specification.subscribe(artifact_indirect_grantee, owner)
# pick one of the concrete artifacts (bug, branch, Git repository,
- # snap, or spec) and subscribe the teams and persons.
+ # snap, spec or ocirecipe) and subscribe the teams and persons.
concrete_artifact, get_pillars, get_subscribers = configure_test(
- bug, branch, gitrepository, snap, specification,
+ bug, branch, gitrepository, snap, specification, ocirecipe,
policy_team_grantee, policy_indirect_grantee,
artifact_team_grantee, owner)
@@ -396,6 +422,7 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
self.assertIn(artifact_indirect_grantee, branch.subscribers)
self.assertIn(artifact_indirect_grantee, gitrepository.subscribers)
self.assertIn(artifact_indirect_grantee, snap.subscribers)
+ self.assertIn(artifact_indirect_grantee, ocirecipe.subscribers)
self.assertIn(artifact_indirect_grantee,
removeSecurityProxy(specification).subscribers)
@@ -409,8 +436,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
concrete_artifact).getDirectSubscribers()
def configure_test(bug, branch, gitrepository, snap, specification,
- policy_team_grantee, policy_indirect_grantee,
- artifact_team_grantee, owner):
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
concrete_artifact = bug
bug.subscribe(policy_team_grantee, owner)
bug.subscribe(policy_indirect_grantee, owner)
@@ -429,8 +457,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
return concrete_artifact.subscribers
def configure_test(bug, branch, gitrepository, snap, specification,
- policy_team_grantee, policy_indirect_grantee,
- artifact_team_grantee, owner):
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
concrete_artifact = branch
branch.subscribe(
policy_team_grantee,
@@ -458,8 +487,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
return concrete_artifact.subscribers
def configure_test(bug, branch, gitrepository, snap, specification,
- policy_team_grantee, policy_indirect_grantee,
- artifact_team_grantee, owner):
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
concrete_artifact = gitrepository
gitrepository.subscribe(
policy_team_grantee,
@@ -487,8 +517,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
return concrete_artifact.subscribers
def configure_test(bug, branch, gitrepository, snap, specification,
- policy_team_grantee, policy_indirect_grantee,
- artifact_team_grantee, owner):
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
concrete_artifact = snap
snap.subscribe(policy_team_grantee, owner)
snap.subscribe(policy_indirect_grantee, owner)
@@ -507,8 +538,9 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
return concrete_artifact.subscribers
def configure_test(bug, branch, gitrepository, snap, specification,
- policy_team_grantee, policy_indirect_grantee,
- artifact_team_grantee, owner):
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
naked_spec = removeSecurityProxy(specification)
naked_spec.subscribe(policy_team_grantee, owner)
naked_spec.subscribe(policy_indirect_grantee, owner)
@@ -521,6 +553,27 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
self._assert_artifact_change_unsubscribes(
change_callback, configure_test)
+ def _assert_ocirecipe_change_unsubscribes(self, change_callback):
+
+ def get_pillars(concrete_artifact):
+ return [concrete_artifact.oci_project.project]
+
+ def get_subscribers(concrete_artifact):
+ return concrete_artifact.subscribers
+
+ def configure_test(bug, branch, gitrepository, snap, specification,
+ ocirecipe, policy_team_grantee,
+ policy_indirect_grantee, artifact_team_grantee,
+ owner):
+ concrete_artifact = ocirecipe
+ ocirecipe.subscribe(policy_team_grantee, owner)
+ ocirecipe.subscribe(policy_indirect_grantee, owner)
+ ocirecipe.subscribe(artifact_team_grantee, owner)
+ return concrete_artifact, get_pillars, get_subscribers
+
+ self._assert_artifact_change_unsubscribes(
+ change_callback, configure_test)
+
def test_change_information_type_branch(self):
def change_information_type(branch):
removeSecurityProxy(branch).information_type = (
@@ -549,6 +602,13 @@ class RemoveArtifactSubscriptionsJobTestCase(TestCaseWithFactory):
self._assert_specification_change_unsubscribes(change_information_type)
+ def test_change_information_type_ocirecipe(self):
+ def change_information_type(ocirecipe):
+ removeSecurityProxy(ocirecipe).information_type = (
+ InformationType.PRIVATESECURITY)
+
+ self._assert_ocirecipe_change_unsubscribes(change_information_type)
+
def test_change_information_type(self):
# Changing the information type of a bug unsubscribes users who can no
# longer see the bug.
Follow ups
