Merge ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master

[Thiago F. Pappacena]

Consider OCIRecipeSubscription on IRemoveArtifactSubscriptionsJobSource

[Colin Watson (cjwatson)]

review: Approve

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 a0e755b..a311aff 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,
@@ -857,12 +854,6 @@ class SharingService:
         getUtility(IAccessArtifactGrantSource).revokeByArtifact(
             artifacts_to_delete, [grantee])
 
-        # 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.