launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/sanity-for-rafa into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/sanity-for-rafa/+merge/120912

We should make certain in reconcile_access_for_artifact() that the pillars specified have AccessPolicies.
-- 
https://code.launchpad.net/~stevenk/launchpad/sanity-for-rafa/+merge/120912
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/sanity-for-rafa into lp:launchpad.

=== modified file 'lib/lp/registry/model/accesspolicy.py'
--- lib/lp/registry/model/accesspolicy.py	2012-08-22 04:58:49 +0000
+++ lib/lp/registry/model/accesspolicy.py	2012-08-23 04:41:26 +0000
@@ -65,14 +65,15 @@
         getUtility(IAccessArtifactSource).delete([artifact])
         return
     [abstract_artifact] = getUtility(IAccessArtifactSource).ensure([artifact])
+    aps = getUtility(IAccessPolicySource).find(
+        (pillar, information_type) for pillar in pillars)
+    if len(pillars) != aps.count():
+        raise ValueError("All pillars require access policies.")
 
     # Now determine the existing and desired links, and make them
     # match.
     apasource = getUtility(IAccessPolicyArtifactSource)
-    wanted_links = set(
-        (abstract_artifact, policy) for policy in
-        getUtility(IAccessPolicySource).find(
-            (pillar, information_type) for pillar in pillars))
+    wanted_links = set((abstract_artifact, policy) for policy in aps)
     existing_links = set([
         (apa.abstract_artifact, apa.policy)
         for apa in apasource.findByArtifact([abstract_artifact])])

=== modified file 'lib/lp/registry/tests/test_accesspolicy.py'
--- lib/lp/registry/tests/test_accesspolicy.py	2012-08-08 07:22:51 +0000
+++ lib/lp/registry/tests/test_accesspolicy.py	2012-08-23 04:41:26 +0000
@@ -725,3 +725,17 @@
         reconcile_access_for_artifact(
             bug, InformationType.USERDATA, [product])
         self.assertPoliciesForBug([(product, InformationType.USERDATA)], bug)
+
+    def test_raises_exception_on_missing_policies(self):
+        # reconcile_access_for_artifact raises an exception if a pillar is
+        # missing an AccessPolicy.
+        product = self.factory.makeProduct()
+        # Creating a product will have created two APs, delete them.
+        aps = getUtility(IAccessPolicySource).findByPillar([product])
+        getUtility(IAccessPolicyGrantSource).revokeByPolicy(aps)
+        for ap in aps:
+            IStore(ap).remove(ap)
+        bug = self.factory.makeBug(target=product)
+        self.assertRaises(
+            ValueError, reconcile_access_for_artifact, bug,
+            InformationType.USERDATA, [product])