[Freeipa-devel] [PATCH] 93 Add custom mapping object for LDAP entry data

[Petr Viktorin]

On 01/16/2013 04:45 PM, Jan Cholasta wrote:
> Hi,
>
> this patch adds initial support for custom LDAP entry objects, as
> described in <http://freeipa.org/page/V3/LDAP_code>.
>
> The LDAPEntry class implements the mapping object. The current version
> works like a dict, plus it has a DN property and validates and
> normalizes attribute names (there is no case preservation yet).
>
> The LDAPEntryCompat class provides compatibility with old code that uses
> (dn, attrs) tuples. The reason why this is a separate class is that our
> code currently has 2 contradicting requirements on the entry object:
> tuple unpacking must work with it (i.e. iter(entry) yields dn and
> attribute dict), but it also must work as an argument to dict
> constructor (i.e. iter(entry) yields attribute names). This class will
> be removed once our code is converted to use LDAPEntry.

With this patch, ldap2 produces LDAPEntryCompat objects. I don't see how 
that brings us closer to converting the codebase to LDAPEntry.
I'd be happier if this patch made both the tuple unpacking and 
dict-style access possible. But perhaps you have a better plan than I 
can see from the patch?


The dict constructor uses the `keys` method, not iteration, so these two 
requirements aren't incompatible -- unless we do specifically require 
that iter(entry) yields attribute names. For example the following will 
work:

     class NotQuiteADict(dict):
         def __iter__(self):
             return iter(('some', 'data'))

     a = NotQuiteADict(a=1, b=2, c=3)
     print dict(a)  # -> {'a': 1, 'c': 3, 'b': 2}
     print list(a)  # -> ['some', 'data']
     print a.keys()  # -> ['a', 'c', 'b']


While overriding a dict's __iter__ in this way is a bit evil, I think 
for our purposes it would be better than introducing a fourth entry class.
When all our code is converted we could just have __iter__ warn or raise 
an exception for a few releases to make sure no one uses it.

In a couple of places we do iterate over the entry to get the keys, but 
not in many, and they're usually well-tested. We just have to use 
`entry.keys()` there.

Would these changes to your patch be OK?



Another issue is that the DN is itself an attribute, so entry.dn and 
entry['dn'] should be synchronized. I guess that's material for another 
patch, though.

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Add-custom-mapping-object-for-LDAP-entry-data.patch
Type: text/x-patch
Size: 11898 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130116/50bce8b3/attachment.bin>