Minor speed tweaks.

2 files changed, 63 insertions, 54 deletions

diff --git a/repoze/bfg/security.py b/repoze/bfg/security.py
index a93c88695..edb7871d8 100644
--- a/repoze/bfg/security.py
+++ b/repoze/bfg/security.py
@@ -80,18 +80,18 @@ class ACLSecurityPolicy(object):
 
             for ace in acl:
                 ace_action, ace_principal, ace_permissions = ace
-                if ace_principal not in principals:
-                    continue
-                for principal in principals:
-                    if ace_principal == principal:
-                        permissions = flatten(ace_permissions)
-                        if permission in permissions:
-                            if ace_action == Allow:
-                                return ACLAllowed(ace, acl, permission,
-                                                  principals, location)
-                            else:
-                                return ACLDenied(ace, acl, permission,
-                                                 principals, location)
+                if ace_principal in principals:
+                    if hasattr(ace_permissions, '__iter__'):
+                        ace_permissions = _flatten(ace_permissions)
+                    else:
+                        ace_permissions = [ace_permissions]
+                    if permission in ace_permissions:
+                        if ace_action == Allow:
+                            return ACLAllowed(ace, acl, permission,
+                                              principals, location)
+                        else:
+                            return ACLDenied(ace, acl, permission,
+                                             principals, location)
 
             # default deny if no ACE matches in the ACL found
             result = ACLDenied(None, acl, permission, principals, location)
@@ -117,16 +117,23 @@ class ACLSecurityPolicy(object):
 
     def principals_allowed_by_permission(self, context, permission):
         for location in lineage(context):
-            acl = getattr(location, '__acl__', None)
-            if acl is not None:
-                allowed = {}
-                for ace_action, ace_principal, ace_permissions in acl:
-                    if ace_action == Allow:
-                        ace_permissions = flatten(ace_permissions)
-                        for ace_permission in ace_permissions:
-                            if ace_permission == permission:
-                                allowed[ace_principal] = True
-                return sorted(allowed.keys())
+            try:
+                acl = location.__acl__
+            except AttributeError:
+                continue
+
+            allowed = {}
+
+            for ace_action, ace_principal, ace_permissions in acl:
+                if ace_action == Allow:
+                    if hasattr(ace_permissions, '__iter__'):
+                        ace_permissions = _flatten(ace_permissions)
+                    else:
+                        ace_permissions = [ace_permissions]
+                    if permission in ace_permissions:
+                        allowed[ace_principal] = True
+            return sorted(allowed.keys())
+
         return []
 
 def get_remoteuser(request):
@@ -288,7 +295,7 @@ class ACLAllowed(ACLPermitsResult):
     as he ``msg`` attribute."""
     boolval = 1
 
-def flatten(x):
+def _flatten(iterable):
     """flatten(sequence) -> list
 
     Returns a single, flat list which contains all elements retrieved
@@ -300,11 +307,6 @@ def flatten(x):
     [1, 2, [3, 4], (5, 6)]
     >>> flatten([[[1,2,3], (42,None)], [4,5], [6], 7, MyVector(8,9,10)])
     [1, 2, 3, 42, None, 4, 5, 6, 7, 8, 9, 10]"""
-    if not hasattr(x, '__iter__'):
-        return [x]
-    return _flatten(x)
-
-def _flatten(iterable):
     result = []
     for el in iterable:
         if hasattr(el, "__iter__"):
diff --git a/repoze/bfg/tests/test_security.py b/repoze/bfg/tests/test_security.py
index d6d653dba..3ee89d9c3 100644
--- a/repoze/bfg/tests/test_security.py
+++ b/repoze/bfg/tests/test_security.py
@@ -38,7 +38,7 @@ class TestACLSecurityPolicy(unittest.TestCase):
         self.assertEqual(result.permission, 'view')
         self.assertEqual(result.context, context)
 
-    def test_permits_no_principals_acl_info_on_context(self):
+    def test_permits_no_principals_empty_acl_info_on_context(self):
         context = DummyContext()
         context.__acl__ = []
         request = DummyRequest({})
@@ -50,7 +50,7 @@ class TestACLSecurityPolicy(unittest.TestCase):
         self.assertEqual(result.permission, 'view')
         self.assertEqual(result.context, context)
 
-    def test_permits_no_principals_withparents_root_has_acl_info(self):
+    def test_permits_no_principals_root_has_empty_acl_info(self):
         context = DummyContext()
         context.__name__ = None
         context.__parent__ = None
@@ -67,7 +67,7 @@ class TestACLSecurityPolicy(unittest.TestCase):
         self.assertEqual(result.permission, 'view')
         self.assertEqual(result.context, context)
 
-    def test_permits_no_principals_withparents_root_allows_everyone(self):
+    def test_permits_no_principals_root_allows_everyone(self):
         context = DummyContext()
         context.__name__ = None
         context.__parent__ = None
@@ -127,27 +127,22 @@ class TestACLSecurityPolicy(unittest.TestCase):
         self.assertEqual(result.context, context)
         self.assertEqual(result.ace, None)
 
-    def test_permits_allow_via_location_parent(self):
-        from repoze.bfg.security import Allow, Authenticated, Everyone
+    def test_permits_allow_twoacl_multiperm(self):
+        from repoze.bfg.security import Allow, Deny, Authenticated, Everyone
         context = DummyContext()
-        context.__parent__ = None
-        context.__name__ = None
-        context.__acl__ = [ (Allow, 'fred', 'read') ]
-        context2 = DummyContext()
-        context2.__parent__ = context
-        context2.__name__ = 'myname'
-
+        acl = [ (Allow, 'fred', ('write', 'view') ), (Deny, 'fred', 'view') ]
+        context.__acl__ = acl
         policy = self._makeOne(lambda *arg: ['fred'])
         request = DummyRequest({})
-        result = policy.permits(context2, request, 'read')
+        result = policy.permits(context, request, 'view')
         self.assertEqual(result, True)
         self.assertEqual(result.principals,
                          set(['fred', Authenticated, Everyone]))
-        self.assertEqual(result.permission, 'read')
+        self.assertEqual(result.permission, 'view')
         self.assertEqual(result.context, context)
-        self.assertEqual(result.ace, ('Allow', 'fred', 'read'))
+        self.assertEqual(result.ace, (Allow, 'fred', ('write', 'view') ))
 
-    def test_permits_multipermission(self):
+    def test_permits_deny_twoacl_multiperm(self):
         from repoze.bfg.security import Allow, Deny, Authenticated, Everyone
         context = DummyContext()
         acl = []
@@ -164,6 +159,26 @@ class TestACLSecurityPolicy(unittest.TestCase):
         self.assertEqual(result.context, context)
         self.assertEqual(result.ace, deny)
 
+    def test_permits_allow_via_location_parent(self):
+        from repoze.bfg.security import Allow, Authenticated, Everyone
+        context = DummyContext()
+        context.__parent__ = None
+        context.__name__ = None
+        context.__acl__ = [ (Allow, 'fred', 'read') ]
+        context2 = DummyContext()
+        context2.__parent__ = context
+        context2.__name__ = 'myname'
+
+        policy = self._makeOne(lambda *arg: ['fred'])
+        request = DummyRequest({})
+        result = policy.permits(context2, request, 'read')
+        self.assertEqual(result, True)
+        self.assertEqual(result.principals,
+                         set(['fred', Authenticated, Everyone]))
+        self.assertEqual(result.permission, 'read')
+        self.assertEqual(result.context, context)
+        self.assertEqual(result.ace, ('Allow', 'fred', 'read'))
+
     def test_permits_deny_byorder(self):
         from repoze.bfg.security import Allow, Deny, Authenticated, Everyone
         context = DummyContext()
@@ -518,16 +533,8 @@ class TestACLDenied(unittest.TestCase):
 
 class TestFlatten(unittest.TestCase):
     def _callFUT(self, item):
-        from repoze.bfg.security import flatten
-        return flatten(item)
-
-    def test_str(self):
-        result = self._callFUT('a')
-        self.assertEqual(result, ['a'])
-
-    def test_unicode(self):
-        result = self._callFUT(u'a')
-        self.assertEqual(result, [u'a'])
+        from repoze.bfg.security import _flatten
+        return _flatten(item)
 
     def test_flat_sequence(self):
         result = self._callFUT([1, 2, 3])