launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Create a new garbo job to populate Branch.access_grants (via AccessArtifactGrants)
-- 
https://code.launchpad.net/~stevenk/launchpad/populate-branch-aag/+merge/112264
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/populate-branch-aag into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2012-06-26 14:23:03 +0000
+++ database/schema/security.cfg	2012-06-27 04:00:26 +0000
@@ -2197,6 +2197,8 @@
 
 [garbo]
 groups=script,read
+public.accessartifact                   = SELECT, INSERT
+public.accessartifactgrant              = 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-22 05:36:22 +0000
+++ lib/lp/scripts/garbo.py	2012-06-27 04:00:26 +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,11 @@
     RevisionCache,
     )
 from lp.hardwaredb.model.hwdb import HWSubmission
+from lp.registry.enums import PRIVATE_INFORMATION_TYPES
+from lp.registry.interfaces.accesspolicy import (
+    IAccessArtifactGrantSource,
+    IAccessArtifactSource,
+    )
 from lp.registry.model.person import Person
 from lp.services.config import config
 from lp.services.database import postgresql
@@ -1137,6 +1143,48 @@
         transaction.commit()
 
 
+class PopulateBranchAccessArtifactGrant(TunableLoop):
+
+    maximum_chunk_size = 5000
+    
+    def __init__(self, log, abort_time=None):
+        super(PopulateBranchAccessArtifactGrant, self).__init__(
+            log, abort_time)
+        self.memcache_key = '%s:branch-populate-aag' % config.instance_name
+        watermark = getUtility(IMemcacheClient).get(self.memcache_key)
+        self.start_at = watermark or 0
+
+    def findBranches(self):
+        return IMasterStore(Branch).find(
+            Branch,
+            Branch.information_type.is_in(PRIVATE_INFORMATION_TYPES),
+            Branch.id >= self.start_at).order_by(Branch.id)
+
+    def isDone(self):
+        return self.findBranches().is_empty()
+
+    def __call__(self, chunk_size):
+        branches = list(self.findBranches()[:chunk_size])
+        artifacts = getUtility(IAccessArtifactSource).ensure(branches)
+        branch_to_artifact = dict([(artifact.branch_id, artifact)
+            for artifact in artifacts])
+        grants = []
+        for branch in branches:
+            artifact = branch_to_artifact[branch.id]
+            bgrants = [(artifact, branchsub.person, branchsub.subscribed_by)
+                for branchsub in branch.subscriptions]
+            if branch.owner not in [grant[1] for grant in bgrants]:
+                bgrants.append((artifact, branch.owner, branch.owner))
+            grants.extend(bgrants)
+        getUtility(IAccessArtifactGrantSource).grant(grants)
+        self.start_at = branches[-1].id + 1
+        result = getUtility(IMemcacheClient).set(
+            self.memcache_key, self.start_at)
+        if not result:
+            self.log.warning('Failed to set start_at in memcache.')
+        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 +1440,7 @@
         DuplicateSessionPruner,
         BugHeatUpdater,
         BugTaskFlattener,
+        PopulateBranchAccessArtifactGrant,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-06-22 05:36:22 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2012-06-27 04:00:26 +0000
@@ -47,7 +47,11 @@
     BranchFormat,
     RepositoryFormat,
     )
-from lp.code.enums import CodeImportResultStatus
+from lp.code.enums import (
+    BranchSubscriptionNotificationLevel,
+    CodeImportResultStatus,
+    CodeReviewNotificationLevel,
+    )
 from lp.code.interfaces.codeimportevent import ICodeImportEventSet
 from lp.code.model.branchjob import (
     BranchJob,
@@ -55,6 +59,7 @@
     )
 from lp.code.model.codeimportevent import CodeImportEvent
 from lp.code.model.codeimportresult import CodeImportResult
+from lp.registry.enums import InformationType
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.scripts.garbo import (
@@ -1149,6 +1154,38 @@
         self.runHourly()
         self.assertEqual((task.id,), get_flat())
 
+    def test_PopulateBranchAccessArtifactGrant(self):
+        # Branches without a access_policy have one set by the job.
+        with dbuser('testadmin'):
+            branch = self.factory.makeBranch()
+
+        def get_access_grants():
+            return IMasterStore(BugTask).execute(
+                'SELECT access_grants 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_grants())
+
+        with dbuser('testadmin'):
+            branch.transitionToInformationType(
+                InformationType.USERDATA, branch.owner, verify_policy=False)
+
+        # Now the branch is USERDATA, it will have no explicit grants.
+        self.assertEqual([], get_access_grants())
+
+        with dbuser('testadmin'):
+            subscriber = self.factory.makePerson()
+            branch.subscribe(
+                subscriber, BranchSubscriptionNotificationLevel.NOEMAIL,
+                None, CodeReviewNotificationLevel.NOEMAIL, branch.owner)
+
+        # The subscriber has been added.
+        self.runHourly()
+        self.assertContentEqual(
+            [branch.owner.id, subscriber.id], get_access_grants())
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer