Merge lp:~stevenk/launchpad/remove-unused-sharing-garbo into lp:launchpad

[Steve Kowalik]

Add a daily garbo job that will trawl all products and remove all access policies that are unused.

Add a daily garbo job that will trawl all products and remove all access policies that are unused. I've tried to minimize the queries used, and make use of the accesspolicy methods.

[William Grant (wgrant)]

122 + allowed_bug_policies = set(
123 + BUG_POLICY_ALLOWED_TYPES.get(product.bug_sharing_policy, []))
124 + allowed_branch_policies = set(
125 + BRANCH_POLICY_ALLOWED_TYPES.get(
126 + product.branch_sharing_policy, []))

You'll need to either exclude those with *_sharing_policy unset, or default to FREE_INFORMATION_TYPES, otherwise all legacy-configured projects will have their builtin APs reaped.

127 + access_polices = set(

Does it really?

131 + apas = getUtility(IAccessPolicyArtifactSource).findByPolicy(
132 + candidate_aps)
133 + used_aps = set([apa.policy for apa in apas])

This is going to load potentially hundreds of thousands of objects, which is probably not what you want.

review: Needs Fixing (code)

[William Grant (wgrant)]

Could you a docstring to the TunableLoop and perhaps some comments describing what it's doing and why? Apart from that it's fine.

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-08-21 20:41:05 +0000
+++ database/schema/security.cfg	2012-08-22 23:30:30 +0000
@@ -2220,6 +2220,8 @@
 
 [garbo]
 groups=script,read
+public.accesspolicy                     = SELECT, DELETE
+public.accesspolicygrant                = SELECT, DELETE
 public.account                          = SELECT, DELETE
 public.answercontact                    = SELECT, DELETE
 public.branch                           = SELECT, UPDATE

=== modified file 'lib/lp/code/model/branchnamespace.py'
--- lib/lp/code/model/branchnamespace.py	2012-08-20 13:26:21 +0000
+++ lib/lp/code/model/branchnamespace.py	2012-08-22 23:30:30 +0000
@@ -8,6 +8,7 @@
     'BranchNamespaceSet',
     'PackageNamespace',
     'PersonalNamespace',
+    'POLICY_ALLOWED_TYPES',
     'ProductNamespace',
     ]
 

=== modified file 'lib/lp/registry/interfaces/accesspolicy.py'
--- lib/lp/registry/interfaces/accesspolicy.py	2012-07-19 03:18:37 +0000
+++ lib/lp/registry/interfaces/accesspolicy.py	2012-08-22 23:30:30 +0000
@@ -229,6 +229,9 @@
             pairs.
         """
 
+    def revokeByPolicy(policies):
+        """Revoke all `IAccessPolicyGrant` for the policies."""
+
 
 class IAccessPolicyGrantFlatSource(Interface):
     """Experimental query utility to search through the flattened schema."""

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2012-08-06 03:47:42 +0000
+++ lib/lp/registry/model/accesspolicy.py	2012-08-22 23:30:30 +0000
@@ -378,6 +378,11 @@
         """See `IAccessPolicyGrantSource`."""
         cls.find(grants).remove()
 
+    @classmethod
+    def revokeByPolicy(cls, policies):
+        """See `IAccessPolicyGrantSource`."""
+        cls.findByPolicy(policies).remove()
+
 
 class AccessPolicyGrantFlat(StormBase):
     __storm_table__ = 'AccessPolicyGrantFlat'

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-08-17 07:18:49 +0000
+++ lib/lp/scripts/garbo.py	2012-08-22 23:30:30 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'DailyDatabaseGarbageCollector',
+    'FrequentDatabaseGarbageCollector',
     'HourlyDatabaseGarbageCollector',
     ]
 
@@ -46,6 +47,9 @@
 
 from lp.answers.model.answercontact import AnswerContact
 from lp.bugs.interfaces.bug import IBugSet
+from lp.bugs.interfaces.bugtarget import (
+    POLICY_ALLOWED_TYPES as BUG_POLICY_ALLOWED_TYPES,
+    )
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
 from lp.bugs.model.bugnotification import BugNotification
@@ -56,6 +60,9 @@
     )
 from lp.code.enums import BranchVisibilityRule
 from lp.code.interfaces.revision import IRevisionSet
+from lp.code.model.branchnamespace import (
+    POLICY_ALLOWED_TYPES as BRANCH_POLICY_ALLOWED_TYPES,
+    )
 from lp.code.model.branchvisibilitypolicy import BranchVisibilityTeamPolicy
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
@@ -64,6 +71,12 @@
     RevisionCache,
     )
 from lp.hardwaredb.model.hwdb import HWSubmission
+from lp.registry.enums import FREE_INFORMATION_TYPES
+from lp.registry.interfaces.accesspolicy import (
+    IAccessPolicyArtifactSource,
+    IAccessPolicyGrantSource,
+    IAccessPolicySource,
+    )
 from lp.registry.model.commercialsubscription import CommercialSubscription
 from lp.registry.model.person import Person
 from lp.registry.model.product import Product
@@ -1054,6 +1067,50 @@
         transaction.commit()
 
 
+class UnusedSharingPolicyPruner(TunableLoop):
+    """Deletes unused AccessPolicy and AccessPolicyGrants for products."""
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(UnusedSharingPolicyPruner, self).__init__(log, abort_time)
+        self.start_at = 1
+        self.store = IMasterStore(Product)
+
+    def findProducts(self):
+        return self.store.find(
+            Product, Product.id >= self.start_at).order_by(Product.id)
+
+    def isDone(self):
+        return self.findProducts().is_empty()
+
+    def __call__(self, chunk_size):
+        products = list(self.findProducts()[:chunk_size])
+        for product in products:
+            allowed_bug_policies = set(
+                BUG_POLICY_ALLOWED_TYPES.get(
+                    product.bug_sharing_policy, FREE_INFORMATION_TYPES))
+            allowed_branch_policies = set(
+                BRANCH_POLICY_ALLOWED_TYPES.get(
+                    product.branch_sharing_policy, FREE_INFORMATION_TYPES))
+            # Fetch all APs, and after filtering out ones that are forbidden
+            # by the bug and branch policies, the APs that have no APAs are
+            # unused and can be deleted.
+            access_policies = set(
+                getUtility(IAccessPolicySource).findByPillar([product]))
+            candidate_aps = access_policies.difference(
+                allowed_bug_policies, allowed_branch_policies)
+            apa_source = getUtility(IAccessPolicyArtifactSource)
+            unused_aps = [
+                ap for ap in candidate_aps
+                if apa_source.findByPolicy([ap]).is_empty()]
+            getUtility(IAccessPolicyGrantSource).revokeByPolicy(unused_aps)
+            for ap in unused_aps:
+                self.store.remove(ap)
+        self.start_at = products[-1].id + 1
+        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.
@@ -1340,8 +1397,9 @@
         ScrubPOFileTranslator,
         SuggestiveTemplatesCacheUpdater,
         POTranslationPruner,
+        UnlinkedAccountPruner,
         UnusedPOTMsgSetPruner,
-        UnlinkedAccountPruner,
+        UnusedSharingPolicyPruner,
         ]
     experimental_tunable_loops = [
         PersonPruner,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-08-17 07:18:49 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-08-22 23:30:30 +0000
@@ -59,6 +59,7 @@
     BranchSharingPolicy,
     BugSharingPolicy,
     )
+from lp.registry.interfaces.accesspolicy import IAccessPolicySource
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProductSet
 from lp.registry.model.product import Product
@@ -1083,6 +1084,21 @@
             self.assertEqual(
                 BugSharingPolicy.PUBLIC, product.bug_sharing_policy)
 
+    def test_UnusedSharingPolicyPruner(self):
+        # UnusedSharingPolicyPruner destroys the unused sharing details.
+        switch_dbuser('testadmin')
+        product = self.factory.makeProduct(
+            branch_sharing_policy=BranchSharingPolicy.PROPRIETARY)
+        ap = self.factory.makeAccessPolicy(pillar=product)
+        self.factory.makeAccessPolicyArtifact(policy=ap)
+        self.factory.makeAccessPolicyGrant(policy=ap)
+        all_aps = getUtility(IAccessPolicySource).findByPillar([product])
+        # There are 3 because two are created implicitly when the product is.
+        self.assertEqual(3, all_aps.count())
+        self.runDaily()
+        all_aps = getUtility(IAccessPolicySource).findByPillar([product])
+        self.assertEqual([ap], list(all_aps))
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer