Martin Kosek wrote:
On Fri, 2012-05-18 at 10:17 -0400, Rob Crittenden wrote:
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.
Yes, but it only works when the truncated flag is raised by the base
LDAP search, i.e. the search for permission objects (which is a case of
your unit test). If the search does not raise the flag and it is set
later in post callback, it is never propagated to the user. Please check
the attached (crappy) test that shows this issue:
======================================================================
FAIL: test_permission[20]: permission_find: Search for permissions by
attr with a limit of 1 (truncated)
----------------------------------------------------------------------
...
AssertionError: assert_deepequal: expected != got.
test_permission[20]: permission_find: Search for permissions by attr
with a limit of 1 (truncated)
expected = True
got = False
path = ('truncated',)
I am not sure how to solve this right, we may need to add a new return
attribute (truncated) to all LDAPSearch post callbacks so that the post
callback can really modify it - something similar we already do with pre
callbacks which are able to change LDAP search filter, scope etc.
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.
We can now also remove the "enumerate" part, "ind" is no longer needed.
Martin
You're right, I'd have sworn I tested that...
The only solution is going to be to have the post_callback return
truncated. This is going to be a rather intrusive change.
rob
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
