launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/populate-branch-ap into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933768 in Launchpad itself: "Update branch to use information_visibility_policy"
  https://bugs.launchpad.net/launchpad/+bug/933768

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/populate-branch-ap/+merge/110950

With the new columns of access_policy and access_grants now glued to branch, we should populate them. They should start slowly populating as branches are created or transitioned to a private information type. Let's help it along with a garbo job. This does not address AccessArtifactGrants, they will be dealt with in a later branch.
-- 
https://code.launchpad.net/~stevenk/launchpad/populate-branch-ap/+merge/110950
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/populate-branch-ap into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-06-18 02:56:14 +0000
+++ database/schema/security.cfg	2012-06-19 05:33:01 +0000
@@ -2194,6 +2194,8 @@
 
 [garbo]
 groups=script,read
+public.accessartifact                   = SELECT, INSERT
+public.accesspolicyartifact             = SELECT, INSERT
 public.account                          = SELECT, DELETE
 public.answercontact                    = SELECT, DELETE
 public.branch                           = SELECT, UPDATE

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-06-12 06:08:30 +0000
+++ lib/lp/scripts/garbo.py	2012-06-19 05:33:01 +0000
@@ -51,6 +51,7 @@
     MAX_SAMPLE_SIZE,
     )
 from lp.code.interfaces.revision import IRevisionSet
+from lp.code.model.branch import Branch
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
 from lp.code.model.revision import (
@@ -58,6 +59,7 @@
     RevisionCache,
     )
 from lp.hardwaredb.model.hwdb import HWSubmission
+from lp.registry.enums import PRIVATE_INFORMATION_TYPES
 from lp.registry.model.person import Person
 from lp.services.config import config
 from lp.services.database import postgresql
@@ -1137,6 +1139,25 @@
         transaction.commit()
 
 
+class PopulateBranchAccessPolicy(TunableLoop):
+
+    maximum_chunk_size = 5000
+
+    def findBranches(self):
+        return IMasterStore(Branch).find(
+            Branch,
+            Branch.information_type.is_in(PRIVATE_INFORMATION_TYPES),
+            SQL("Branch.access_policy IS NULL"))
+
+    def isDone(self):
+        return self.findBranches().is_empty()
+
+    def __call__(self, chunk_size):
+        for branch in self.findBranches()[:chunk_size]:
+            branch._reconcileAccess()
+        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.
@@ -1392,6 +1413,7 @@
         DuplicateSessionPruner,
         BugHeatUpdater,
         BugTaskFlattener,
+        PopulateBranchAccessPolicy,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-06-12 06:08:30 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-06-19 05:33:01 +0000
@@ -55,6 +55,8 @@
     )
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
+from lp.registry.enums import InformationType
+from lp.registry.interfaces.accesspolicy import IAccessPolicyArtifactSource
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.scripts.garbo import (
@@ -1149,6 +1151,30 @@
         self.runHourly()
         self.assertEqual((task.id,), get_flat())
 
+    def test_PopulateBranchAccessPolicy(self):
+        # Branches without a access_policy have one set by the job.
+        with dbuser('testadmin'):
+            branch = self.factory.makeBranch()
+
+        def get_access_policy():
+            return IMasterStore(BugTask).execute(
+                'SELECT access_policy FROM branch WHERE id = ?',
+                (branch.id,)).get_one()[0]
+
+        # The branch is public, so running the garbo job will have no effect.
+        self.runHourly()
+        self.assertIs(None, get_access_policy())
+
+        with dbuser('testadmin'):
+            branch.transitionToInformationType(
+                InformationType.USERDATA, branch.owner, verify_policy=False)
+        access_artifact = self.factory.makeAccessArtifact(concrete=branch)
+        apas = getUtility(IAccessPolicyArtifactSource).findByArtifact(
+            (access_artifact,))
+        # Now it will be set.
+        self.runHourly()
+        self.assertEqual(apas[0].policy.id, get_access_policy())
+        
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer