Merge pull request #12 from zopefoundation/100coverage

100% coverage

.coveragerc

@@ -1,12 +1,8 @@
 [run]
-branch = True
 source = zope.securitypolicy
 
 [report]
-precision = 2
-
-[paths]
-source =
-    src/
-    .tox/py*/lib/python*/site-packages/
-    .tox/pypy*/site-packages/
+exclude_lines =
+    pragma: no cover
+    raise AssertionError
+    raise NotImplementedError

CHANGES.rst

@@ -10,6 +10,16 @@ Changes
 
 - Drop support for ``python setup.py test``.
 
+- Make ``SecurityMap`` and ``AnnotationGrantInfo`` have proper truth
+  behaviour on Python 3; previously they were always true.
+
+- Make ``AnnotationGrantInfo`` consistently return lists instead of
+  dict views on Python 3.
+
+- Make ``AnnotationSecurityMap`` (and objects derived from it, such as
+  ``AnnotationPrincipalPermissionManager`` and the role managers) more
+  efficient when adding or removing cells before they have been
+  persisted. They now avoid some unnecessary object copying.
 
 4.2.0 (2017-08-24)
 ------------------

src/zope/securitypolicy/grantinfo.py

@@ -36,42 +36,35 @@
 @implementer(IGrantInfo)
 class AnnotationGrantInfo(object):
 
-    prinper = prinrole = permrole = {}
-
     def __init__(self, context):
         self._context = context
-        annotations = IAnnotations(context, None)
-        if annotations is not None:
+        annotations = IAnnotations(context, {})
 
-            prinper = annotations.get(prinperkey)
-            if prinper is not None:
-                self.prinper = prinper._bycol  # by principals
+        # by principals
+        prinper = annotations.get(prinperkey)
+        self.prinper = prinper._bycol if prinper is not None else {}
 
-            prinrole = annotations.get(prinrolekey)
-            if prinrole is not None:
-                self.prinrole = prinrole._bycol  # by principals
+        # by principals
+        prinrole = annotations.get(prinrolekey)
+        self.prinrole = prinrole._bycol if prinrole is not None else {}
 
-            roleper = annotations.get(rolepermkey)
-            if roleper is not None:
-                self.permrole = roleper._byrow  # by permission
+        # by permission
+        roleper = annotations.get(rolepermkey)
+        self.permrole = roleper._byrow if roleper is not None else {}
 
-    def __nonzero__(self):
+    def __bool__(self):
         return bool(self.prinper or self.prinrole or self.permrole)
 
+    __nonzero__ = __bool__
+
     def principalPermissionGrant(self, principal, permission):
-        prinper = self.prinper.get(principal)
-        if prinper:
-            return prinper.get(permission, Unset)
-        return Unset
+        prinper = self.prinper.get(principal, {})
+        return prinper.get(permission, Unset)
 
     def getRolesForPermission(self, permission):
-        permrole = self.permrole.get(permission)
-        if permrole:
-            return permrole.items()
-        return ()
+        permrole = self.permrole.get(permission, {})
+        return list(permrole.items())
 
     def getRolesForPrincipal(self, principal):
-        prinrole = self.prinrole.get(principal)
-        if prinrole:
-            return prinrole.items()
-        return ()
+        prinrole = self.prinrole.get(principal, {})
+        return list(prinrole.items())

src/zope/securitypolicy/principalpermission.py

@@ -112,7 +112,7 @@ def getPrincipalsAndPermissions(self):
 # simpler.
 try:
     from zope.testing.cleanup import addCleanUp
-except ImportError:
+except ImportError: # pragma: no cover
     pass
 else:
     addCleanUp(principalPermissionManager._clear)

src/zope/securitypolicy/principalrole.py

@@ -101,7 +101,7 @@ def getPrincipalsAndRoles(self):
 # simpler.
 try:
     from zope.testing.cleanup import addCleanUp
-except ImportError:
+except ImportError: # pragma: no cover
     pass
 else:
     addCleanUp(principalRoleManager._clear)

src/zope/securitypolicy/rolepermission.py

@@ -104,7 +104,7 @@ def getRolesAndPermissions(self):
 # simpler.
 try:
     from zope.testing.cleanup import addCleanUp
-except ImportError:
+except ImportError: # pragma: no cover
     pass
 else:
     addCleanUp(rolePermissionManager._clear)

src/zope/securitypolicy/securitymap.py

@@ -30,6 +30,8 @@ def _clear(self):
     def __nonzero__(self):
         return bool(self._byrow)
 
+    __bool__ = __nonzero__
+
     def addCell(self, rowentry, colentry, value):
         # setdefault may get expensive if an empty mapping is
         # expensive to create, for PersistentDict for instance.
@@ -146,7 +148,7 @@ def _changed(self):
         if isinstance(map, PersistentSecurityMap):
             map._p_changed = 1
         else:
-            map = PersistentSecurityMap()
+            map = self.map = PersistentSecurityMap()
             map._byrow = self._byrow
             map._bycol = self._bycol
             annotations = IAnnotations(self._context)

src/zope/securitypolicy/tests/test_annotationprincipalpermissionmanager.py

@@ -144,11 +144,3 @@ def testManyPrincipalsOnePermission(self):
         self.assertEqual(len(principals), 2)
         self.assertTrue((prin1, Allow) in principals)
         self.assertTrue((prin2, Deny) in principals)
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_annotationprincipalrolemanager.py

@@ -140,11 +140,3 @@ def testPrincipalsAndRoles(self):
         self.assertTrue((role1, prin1, Allow) in principalsAndRoles)
         self.assertTrue((role1, prin2, Allow) in principalsAndRoles)
         self.assertTrue((role2, prin1, Allow) in principalsAndRoles)
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_annotationrolepermissionmanager.py

@@ -89,11 +89,3 @@ def testNormal(self):
 
         self.assertEqual(mgr.getSetting(self.read, self.peon), Unset)
         self.assertEqual(mgr.getSetting(self.read, self.peon, 1), 1)
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_grantinfo.py

@@ -0,0 +1,116 @@
+##############################################################################
+#
+# Copyright (c) 2018 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+
+import unittest
+
+from zope import interface
+from zope.annotation.interfaces import IAnnotations
+
+from zope.securitypolicy import grantinfo
+
+# pylint:disable=protected-access
+
+class Manager(object):
+
+    def __init__(self):
+        self._bycol = {}
+        self._byrow = {}
+
+
+@interface.implementer(IAnnotations)
+class Annotated(dict):
+    "Annotations"
+
+
+class TestAnnotationGrantInfo(unittest.TestCase):
+
+    def _makeContext(self):
+        # Return a populated context with all three role managers
+        prinper = Manager()
+        prinrole = Manager()
+        roleperm = Manager()
+
+        context = Annotated()
+        context[grantinfo.prinperkey] = prinper
+        context[grantinfo.prinrolekey] = prinrole
+        context[grantinfo.rolepermkey] = roleperm
+
+        return context, prinper, prinrole, roleperm
+
+    def _makeOne(self, context):
+        return grantinfo.AnnotationGrantInfo(context)
+
+    def test_no_annotations(self):
+        # If we provide a context that has no IAnnotations,
+        # we get defaults
+
+        info = self._makeOne(None)
+
+        self.assertFalse(info)
+        self.assertEqual(grantinfo.Unset,
+                         info.principalPermissionGrant(None, None))
+        self.assertEqual([],
+                         info.getRolesForPermission(None))
+        self.assertEqual([],
+                         info.getRolesForPrincipal(None))
+
+
+    def test_manager_attributes(self):
+        context, prinper, prinrole, roleperm = self._makeContext()
+        info = self._makeOne(context)
+
+        self.assertIs(info.prinper, prinper._bycol)
+        self.assertIs(info.prinrole, prinrole._bycol)
+        self.assertIs(info.permrole, roleperm._byrow)
+
+
+    def test_principal_permission_grant(self):
+        context, prinper, _, _ = self._makeContext()
+        grants = prinper._bycol["principal"] = {}
+        grant = grants["permission"] = object()
+
+        info = self._makeOne(context)
+
+        self.assertIs(grant,
+                      info.principalPermissionGrant("principal", "permission"))
+
+        self.assertIs(grantinfo.Unset,
+                      info.principalPermissionGrant("principal", "other permission"))
+
+        self.assertIs(grantinfo.Unset,
+                      info.principalPermissionGrant("other principal", "other permission"))
+
+    def test_roles_for_permission(self):
+        context, _, _, roleperm = self._makeContext()
+        grants = roleperm._byrow["permission"] = {}
+        grants['key'] = 'value'
+
+        info = self._makeOne(context)
+
+        self.assertEqual([('key', 'value')],
+                         info.getRolesForPermission('permission'))
+        self.assertEqual([],
+                         info.getRolesForPermission("other permission"))
+
+    def test_roles_for_principal(self):
+        context, _, prinper, _ = self._makeContext()
+        grants = prinper._bycol["principal"] = {}
+        grants['key'] = 'value'
+
+        info = self._makeOne(context)
+
+        self.assertEqual([('key', 'value')],
+                         info.getRolesForPrincipal('principal'))
+        self.assertEqual([],
+                         info.getRolesForPrincipal("other principal"))

src/zope/securitypolicy/tests/test_principalpermissionmanager.py

@@ -145,11 +145,3 @@ def testManyPrincipalsOnePermission(self):
         self.assertEqual(len(principals), 2)
         self.assertTrue((prin1, Allow) in principals)
         self.assertTrue((prin2, Deny) in principals)
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_principalrolemanager.py

@@ -134,11 +134,3 @@ def testPrincipalsAndRoles(self):
         self.assertTrue((role1, prin1, Allow) in principalsAndRoles)
         self.assertTrue((role1, prin2, Allow) in principalsAndRoles)
         self.assertTrue((role2, prin1, Allow) in principalsAndRoles)
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_role.py

@@ -23,6 +23,3 @@ def test_suite():
     return unittest.TestSuite((
         DocTestSuite('zope.securitypolicy.role', checker=testing.checker),
     ))
-
-if __name__ == '__main__':
-    unittest.main(defaultTest='test_suite')

src/zope/securitypolicy/tests/test_rolepermissionmanager.py

@@ -57,6 +57,8 @@ def testRolePermission(self):
                          [(role, Allow)])
         self.assertEqual(manager.getPermissionsForRole(role),
                          [(permission, Allow)])
+        self.assertEqual(manager.getRolesAndPermissions(),
+                         [('APerm', 'ARole', Allow)])
 
     def testManyPermissionsOneRole(self):
         perm1 = definePermission('Perm One', 'P1').id
@@ -121,11 +123,3 @@ def test_invalidRole(self):
         self.assertRaises(ValueError,
                           manager.grantPermissionToRole, perm1, 'role1'
                           )
-
-
-def test_suite():
-    loader = unittest.TestLoader()
-    return loader.loadTestsFromTestCase(Test)
-
-if __name__ == '__main__':
-    unittest.TextTestRunner().run(test_suite())

src/zope/securitypolicy/tests/test_securitydirectives.py

@@ -291,15 +291,3 @@ def test_RolePrincipalMap(self):
 
         self.assertEqual(len(roles), 1)
         self.assertTrue(("zope.Bar", Deny) in roles)
-
-
-def test_suite():
-    return unittest.TestSuite((
-        unittest.makeSuite(TestRoleDirective),
-        unittest.makeSuite(TestSecurityGrantMapping),
-        unittest.makeSuite(TestSecurityGrantAllMapping),
-        unittest.makeSuite(TestSecurityDenyMapping),
-    ))
-
-if __name__ == '__main__':
-    unittest.main()