launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1067616 in Launchpad itself: "Blueprints could use a denormalized schema for improved performance"
  https://bugs.launchpad.net/launchpad/+bug/1067616

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

Write a garbo job to backfill Specification.access_{policy,grants} for all private blueprints.
-- 
https://code.launchpad.net/~stevenk/launchpad/populate-specification-aag/+merge/143436
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/populate-specification-aag into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-12-14 03:19:41 +0000
+++ lib/lp/scripts/garbo.py	2013-01-16 04:35:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database garbage collection."""
@@ -48,6 +48,8 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.answers.model.answercontact import AnswerContact
+from lp.app.enums import PRIVATE_INFORMATION_TYPES
+from lp.blueprints.model.specification import Specification
 from lp.bugs.interfaces.bug import IBugSet
 from lp.bugs.model.bug import Bug
 from lp.bugs.model.bugattachment import BugAttachment
@@ -104,6 +106,7 @@
 from lp.services.librarian.model import TimeLimitedToken
 from lp.services.log.logger import PrefixFilter
 from lp.services.looptuner import TunableLoop
+from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.oauth.model import OAuthNonce
 from lp.services.openid.model.openidconsumer import OpenIDConsumerNonce
 from lp.services.propertycache import cachedproperty
@@ -1335,6 +1338,40 @@
         transaction.commit()
 
 
+class PopulateSpecificationAccessPolicy(TunableLoop):
+
+    maximum_chunk_size = 5000
+
+    def __init__(self, log, abort_time=None):
+        super(PopulateSpecificationAccessPolicy, self).__init__(
+            log, abort_time)
+        self.memcache_key = '%s:spec-populate-ap' % config.instance_name
+        watermark = getUtility(IMemcacheClient).get(self.memcache_key)
+        self.start_at = watermark or 0
+
+    def findSpecifications(self):
+        return IMasterStore(Specification).find(
+            Specification,
+            Specification.information_type.is_in(PRIVATE_INFORMATION_TYPES),
+            SQL("Specification.access_policy IS NULL"),
+            Specification.id >= self.start_at).order_by(Specification.id)
+
+    def isDone(self):
+        return self.findSpecifications().is_empty()
+
+    def __call__(self, chunk_size):
+        for specification in self.findSpecifications()[:chunk_size]:
+            specification._reconcileAccess()
+            IMasterStore(Specification).execute(
+                'SELECT specification_denorm_access(?)', (specification.id,))
+            self.start_at = specification.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.
@@ -1590,6 +1627,7 @@
         UnusedSessionPruner,
         DuplicateSessionPruner,
         BugHeatUpdater,
+        PopulateSpecificationAccessPolicy,
         ]
     experimental_tunable_loops = []
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2012-12-14 03:19:41 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2013-01-16 04:35:26 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the database garbage collector."""
@@ -38,6 +38,7 @@
 
 from lp.answers.model.answercontact import AnswerContact
 from lp.app.enums import InformationType
+from lp.blueprints.model.specification import Specification
 from lp.bugs.model.bugnotification import (
     BugNotification,
     BugNotificationRecipient,
@@ -57,8 +58,12 @@
 from lp.registry.enums import (
     BranchSharingPolicy,
     BugSharingPolicy,
-    )
-from lp.registry.interfaces.accesspolicy import IAccessPolicySource
+    SpecificationSharingPolicy,
+    )
+from lp.registry.interfaces.accesspolicy import (
+    IAccessPolicyArtifactSource,
+    IAccessPolicySource,
+    )
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.model.commercialsubscription import CommercialSubscription
@@ -121,7 +126,10 @@
     TestCase,
     TestCaseWithFactory,
     )
-from lp.testing.dbuser import switch_dbuser
+from lp.testing.dbuser import (
+    dbuser,
+    switch_dbuser,
+    )
 from lp.testing.layers import (
     DatabaseLayer,
     LaunchpadScriptLayer,
@@ -1273,6 +1281,36 @@
             'PopulateLatestPersonSourcePackageReleaseCache')
         self.assertEqual(spph_2.id, job_data['last_spph_id'])
 
+    def test_PopulateSpecificationAccessPolicy(self):
+        # Specifications without a access_policy have one set by the job.
+        with dbuser('testadmin'):
+            specification = self.factory.makeSpecification()
+            product = removeSecurityProxy(specification.product)
+            product.specification_sharing_policy = (
+                SpecificationSharingPolicy.PUBLIC_OR_PROPRIETARY)
+            self.factory.makeAccessPolicy(pillar=product)
+
+        def get_access_policy():
+            return IMasterStore(Specification).execute(
+                'SELECT access_policy FROM specification WHERE id = ?',
+                (specification.id,)).get_one()[0]
+
+        # The specification is public, so running the garbo job will have no
+        # effect.
+        self.runHourly()
+        self.assertIs(None, get_access_policy())
+
+        with dbuser('testadmin'):
+            specification.transitionToInformationType(
+                InformationType.PROPRIETARY, specification.owner)
+        access_artifact = self.factory.makeAccessArtifact(
+            concrete=specification)
+        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