launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/product-aps-set into lp:launchpad with lp:~wgrant/launchpad/product-aps-db as a prerequisite.

Commit message:
Update the Product.access_policies cache on creation and policy changes.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1452949 in Launchpad itself: "Timeout error while trying to access team site"
  https://bugs.launchpad.net/launchpad/+bug/1452949

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/product-aps-set/+merge/263062

Update the Product.access_policies cache on creation and policy changes. The cache is null unless the project has a proprietary information type, in which case it includes the IDs of AccessPolicies with proprietary information types.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/product-aps-set into lp:launchpad.

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2015-06-24 21:14:20 +0000
+++ lib/lp/registry/model/product.py	2015-06-26 06:42:59 +0000
@@ -38,7 +38,9 @@
 from storm.locals import (
     And,
     Desc,
+    Int,
     Join,
+    List,
     Not,
     Or,
     Select,
@@ -439,6 +441,11 @@
         name='remote_product', allow_none=True, default=None)
     vcs = EnumCol(enum=VCSType, notNull=False)
 
+    # Cache of AccessPolicy.ids that convey launchpad.LimitedView.
+    # Unlike artifacts' cached access_policies, an AccessArtifactGrant
+    # to an artifact in the policy is sufficient for access.
+    access_policies = List(type=Int())
+
     @property
     def date_next_suggest_packaging(self):
         """See `IProduct`
@@ -582,6 +589,7 @@
                 self.setSpecificationSharingPolicy(
                     specification_policy_default[value])
             self._ensurePolicies([value])
+            self._cacheAccessPolicies()
 
     information_type = property(_get_information_type, _set_information_type)
 
@@ -753,6 +761,7 @@
                 raise ProprietaryProduct(
                     "The project is %s." % self.information_type.title)
         self._ensurePolicies(allowed_types[var])
+        self._cacheAccessPolicies()
 
     def setBranchSharingPolicy(self, branch_sharing_policy):
         """See `IProductEditRestricted`."""
@@ -815,6 +824,23 @@
             grants.append((p, self.owner, self.owner))
         getUtility(IAccessPolicyGrantSource).grant(grants)
 
+    def _cacheAccessPolicies(self):
+        # Update the cache of AccessPolicy.ids for which an
+        # AccessPolicyGrant or AccessArtifactGrant is sufficient to
+        # convey launchpad.LimitedView on this Product.
+        #
+        # We only need a cache for proprietary types, and it only
+        # includes proprietary policies in case a policy like Private
+        # Security was somehow left around when a project was
+        # transitioned to Proprietary.
+        if self.information_type in PROPRIETARY_INFORMATION_TYPES:
+            self.access_policies = [
+                policy.id for policy in
+                getUtility(IAccessPolicySource).findByPillar([self])
+                if policy.type in PROPRIETARY_INFORMATION_TYPES]
+        else:
+            self.access_policies = None
+
     def _pruneUnusedPolicies(self):
         allowed_bug_types = set(
             BUG_POLICY_ALLOWED_TYPES.get(
@@ -840,6 +866,7 @@
             and apa_source.findByPolicy([ap]).is_empty()]
         getUtility(IAccessPolicyGrantSource).revokeByPolicy(unused_aps)
         ap_source.delete([(ap.pillar, ap.type) for ap in unused_aps])
+        self._cacheAccessPolicies()
 
     @cachedproperty
     def commercial_subscription(self):

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2015-06-25 07:39:40 +0000
+++ lib/lp/registry/tests/test_product.py	2015-06-26 06:42:59 +0000
@@ -527,6 +527,40 @@
             SpecificationSharingPolicy.PROPRIETARY,
             product.specification_sharing_policy)
 
+    def test_cacheAccessPolicies(self):
+        # Product.access_policies is a list caching AccessPolicy.ids for
+        # which an AccessPolicyGrant or AccessArtifactGrant gives a
+        # principal LimitedView on the Product.
+        aps = getUtility(IAccessPolicySource)
+
+        # Public projects don't need a cache.
+        product = self.factory.makeProduct()
+        naked_product = removeSecurityProxy(product)
+        self.assertContentEqual(
+            [InformationType.USERDATA, InformationType.PRIVATESECURITY],
+            [p.type for p in aps.findByPillar([product])])
+        self.assertIs(None, naked_product.access_policies)
+
+        # A private project normally just allows the Proprietary policy,
+        # even if there is still another policy like Private Security.
+        naked_product.information_type = InformationType.PROPRIETARY
+        [prop_policy] = aps.find([(product, InformationType.PROPRIETARY)])
+        self.assertEqual([prop_policy.id], naked_product.access_policies)
+
+        # If we switch it back to public, the cache is no longer
+        # required.
+        naked_product.information_type = InformationType.PUBLIC
+        self.assertIs(None, naked_product.access_policies)
+
+        # Projects can also be Embargoed because of reasons. Since they
+        # can have both Proprietary and Embargoed artifacts, and someone
+        # who can see either needs LimitedView on the pillar they're on,
+        # both policies are permissible.
+        naked_product.information_type = InformationType.EMBARGOED
+        [emb_policy] = aps.find([(product, InformationType.EMBARGOED)])
+        self.assertContentEqual(
+            [prop_policy.id, emb_policy.id], naked_product.access_policies)
+
     def test_checkInformationType_bug_supervisor(self):
         # Bug supervisors of proprietary products must not have inclusive
         # membership policies.