Merge ~cjwatson/launchpad:remove-storm-workarounds into launchpad:master

[Colin Watson]

Remove workarounds for Storm reference cycles

https://code.launchpad.net/~cjwatson/storm/more-weakrefs/+merge/365639
fixed a long-standing Storm bug that sometimes resulted in GC cycles.
We've had this fix deployed in Launchpad for a while with no problems,
so it now makes sense to remove workarounds for it in favour of more
natural code.

[William Grant (wgrant)]

review: Approve (code)

diff --git a/lib/lp/registry/model/product.py b/lib/lp/registry/model/product.py
index 97c6138..8b47075 100644
--- a/lib/lp/registry/model/product.py
+++ b/lib/lp/registry/model/product.py
@@ -42,6 +42,8 @@ from storm.expr import (
     SQL,
     )
 from storm.locals import (
+    Int,
+    List,
     Store,
     Unicode,
     )
@@ -363,6 +365,11 @@ class Product(SQLBase, BugTargetBase, MakesAnnouncements,
         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 displayname(self):
         return self.display_name
@@ -759,18 +766,12 @@ class Product(SQLBase, BugTargetBase, MakesAnnouncements,
         # Security was somehow left around when a project was
         # transitioned to Proprietary.
         if self.information_type in PROPRIETARY_INFORMATION_TYPES:
-            policy_ids = [
+            self.access_policies = [
                 policy.id for policy in
                 getUtility(IAccessPolicySource).find(
                     [(self, type) for type in PROPRIETARY_INFORMATION_TYPES])]
         else:
-            policy_ids = None
-        # XXX wgrant 2015-06-30: We must set this manually, as Storm's
-        # MutableValueVariable causes a PostgresConnection to be
-        # uncollectable in some circumstances involving a Store.reset.
-        Store.of(self).execute(
-            "UPDATE Product SET access_policies = ? WHERE id = ?",
-            (policy_ids, self.id))
+            self.access_policies = None
 
     def _pruneUnusedPolicies(self):
         allowed_bug_types = set(
diff --git a/lib/lp/registry/tests/test_product.py b/lib/lp/registry/tests/test_product.py
index 7387ed2..6dc0e96 100644
--- a/lib/lp/registry/tests/test_product.py
+++ b/lib/lp/registry/tests/test_product.py
@@ -495,29 +495,24 @@ class TestProduct(TestCaseWithFactory):
         # principal LimitedView on the Product.
         aps = getUtility(IAccessPolicySource)
 
-        def get_aps(product):
-            return Store.of(product).execute(
-                "SELECT access_policies FROM product WHERE id = ?",
-                (product.id,)).get_one()[0]
-
         # 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, get_aps(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], get_aps(naked_product))
+        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, get_aps(naked_product))
+        self.assertIs(None, naked_product.access_policies)
 
         # Proprietary projects can have both Proprietary and Embargoed
         # artifacts, and someone who can see either needs LimitedView on
@@ -528,7 +523,7 @@ class TestProduct(TestCaseWithFactory):
             BugSharingPolicy.EMBARGOED_OR_PROPRIETARY)
         [emb_policy] = aps.find([(product, InformationType.EMBARGOED)])
         self.assertContentEqual(
-            [prop_policy.id, emb_policy.id], get_aps(naked_product))
+            [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