Rob Crittenden wrote:
Martin Kosek wrote:
On Thu, 2012-05-17 at 16:11 -0400, Rob Crittenden wrote:
We do two searches when looking for permissions. One within the
permission object itself and a second in the ACIs. We weren't enforcing
a sizelimit on either search.

rob

This returns the right result, but I don't think it is right with
respect to "truncated" flag because of several reasons:

1) You manipulate and set "truncated" flag in post_callback but this
won't affect the flag in the returned result because the new value is
not propagated outside of the post_callback function. I.e. truncated
flag will be set correctly only when it was raised during original
permission_find.

Truncated is still honored as expected. I even added a test case for this.

2) The part with "ind" is strange:

+ # enforce --sizelimit
+ if len(entries) == max_entries:
+ if ind + 1< len(results):
+ truncated = True
+ break

I think it would be much easier to just do

...
if (dn, permission) not in entries:
if len(entries)< max_entries:
entries.append((dn, permission))
else:
truncated = True
break

Otherwise you would rise "truncated" even when the rest of "results"
does not contain relevant entries that would have not been added anyway.

Yes, that makes sense.

And now updated patch.

rob
>From 9c68b2f7be524c1fde382439f05042aa48cde8cd Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Thu, 17 May 2012 16:03:24 -0400
Subject: [PATCH] Enforce sizelimit in permission-find.

We actually perform two searches in permission-find. The first looks
for matches within the permission object itself. The second looks at
matches in the underlying aci.

We need to break out in two places. The first is if we find enough
matches in the permission itself. The second when we are appending
matches from acis.

https://fedorahosted.org/freeipa/ticket/2322
---
 ipalib/plugins/permission.py                |   17 ++++++++--
 tests/test_xmlrpc/test_permission_plugin.py |   46 +++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 891970d9df80918502d4f348bdad926ebd244de7..c0133b6a49b852843ef5dd631bb762decc523bcc 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -377,6 +377,15 @@ class permission_find(LDAPSearch):
                         attrs[attr] = aci[attr]
             except errors.NotFound:
                 self.debug('ACI not found for %s' % attrs['cn'][0])
+        if truncated:
+            # size/time limit met, no need to search acis
+            return
+
+        if 'sizelimit' in options:
+            max_entries = options['sizelimit']
+        else:
+            config = ldap.get_ipa_config()[1]
+            max_entries = config['ipasearchrecordslimit']
 
         # Now find all the ACIs that match. Once we find them, add any that
         # aren't already in the list along with their permission info.
@@ -392,7 +401,7 @@ class permission_find(LDAPSearch):
         truncated = truncated or aciresults['truncated']
         results = aciresults['result']
 
-        for aci in results:
+        for ind, aci in enumerate(results):
             found = False
             if 'permission' in aci:
                 for entry in entries:
@@ -405,7 +414,11 @@ class permission_find(LDAPSearch):
                     dn = permission['dn']
                     del permission['dn']
                     if (dn, permission) not in entries:
-                        entries.append((dn, permission))
+                       if len(entries) < max_entries:
+                           entries.append((dn, permission))
+                       else:
+                           truncated = True
+                           break
 
 api.register(permission_find)
 
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index f54e20462c3c26a0a1cd98a0a75a310bacfc62de..30899730a28076e08f0efde55890c7c5d5b0b149 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -355,6 +355,52 @@ class test_permission(Declarative):
 
 
         dict(
+            desc='Search for %r with a limit of 1 (truncated)' % permission1,
+            command=('permission_find', [permission1], dict(sizelimit=1)),
+            expected=dict(
+                count=1,
+                truncated=True,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': lambda x: DN(x) == permission1_dn,
+                        'cn': [permission1],
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r with a limit of 2' % permission1,
+            command=('permission_find', [permission1], dict(sizelimit=2)),
+            expected=dict(
+                count=2,
+                truncated=False,
+                summary=u'2 permissions matched',
+                result=[
+                    {
+                        'dn': lambda x: DN(x) == permission1_dn,
+                        'cn': [permission1],
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                    },
+                    {
+                        'dn': lambda x: DN(x) == permission2_dn,
+                        'cn': [permission2],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
             desc='Update %r' % permission1,
             command=(
                 'permission_mod', [permission1], dict(permissions=u'read', memberof=u'ipausers')
-- 
1.7.10.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to