[Freeipa-devel] [PATCH 49/49] ticket #1870 - subclass SimpleLDAPObject

Rich Megginson rmeggins at redhat.com
Thu Sep 29 21:06:56 UTC 2011


On 09/28/2011 01:27 PM, John Dennis wrote:
> We use convenience types (classes) in IPA which make working with LDAP
> easier and more robust. It would be really nice if the basic python-ldap
> library understood our utility types and could accept them as parameters
> to the basic ldap functions and/or the basic ldap functions returned our
> utility types.
>
> Normally such a requirement would trivially be handled in an object-
> oriented language (which Python is) by subclassing to extend and modify
> the functionality. For some reason we didn't do this with the python-ldap
> classes.
>
> python-ldap objects are primarily used in two different places in our
> code, ipaserver.ipaldap.py for the IPAdmin class and in
> ipaserver/plugins/ldap2.py for the ldap2 class's .conn member.
>
> In IPAdmin we use a IPA utility class called Entry to make it easier to
> use the results returned by LDAP. The IPAdmin class is derived from
> python-ldap.SimpleLDAPObject. But for some reason when we added the
> support for the use of the Entry class in SimpleLDAPObject we didn't
> subclass SimpleLDAPObject and extend it for use with the Entry class as
> would be the normal expected methodology in an object-oriented language,
> rather we used an obscure feature of the Python language to override all
> methods of the SimpleLDAPObject class by wrapping those class methods in
> another function call. The reason why this isn't a good approach is:
>
> * It violates object-oriented methodology.
>
> * Other classes cannot be derived and inherit the customization (because
> the method wrapping occurs in a class instance, not within the class
> type).
>
> * It's non-obvious and obscure
>
> * It's inefficient.
>
> Here is a summary of what the code was doing:
>
> It iterated over every member of the SimpleLDAPObject class and if it was
> callable it wrapped the method. The wrapper function tested the name of
> the method being wrapped, if it was one of a handful of methods we wanted
> to customize we modified a parameter and called the original method. If
> the method wasn't of interest to use we still wrapped the method.
>
> It was inefficient because every non-customized method (the majority)
> executed a function call for the wrapper, the wrapper during run-time used
> logic to determine if the method was being overridden and then called the
> original method. So every call to ldap was doing extra function calls and
> logic processing which for the majority of cases produced nothing useful
> (and was non-obvious from brief code reading some methods were being
> overridden).
>
> Object-orientated languages have support built in for calling the right
> method for a given class object that do not involve extra function call
> overhead to realize customized class behaviour. Also when programmers look
> for customized class behaviour they look for derived classes. They might
> also want to utilize the customized class as the base class for their use.
>
> Also the wrapper logic was fragile, it did things like: if the method name
> begins with "add" I'll unconditionally modify the first and second
> argument. It would be some much cleaner if the "add", "add_s", etc.
> methods were overridden in a subclass where the logic could be seen and
> where it would apply to only the explicit functions and parameters being
> overridden.
>
> Also we would really benefit if there were classes which could be used as
> a base class which had specific ldap customization.
>
> At the moment our ldap customization needs are:
>
> 1) Support DN objects being passed to ldap operations
>
> 2) Support Entry&  Entity objects being passed into and returned from
> ldap operations.
>
> We want to subclass the ldap SimpleLDAPObject class, that is the base
> ldap class with all the ldap methods we're using. IPASimpleLDAPObject
> class would subclass SimpleLDAPObject class which knows about DN
> objects (and possilby other IPA specific types that are universally
> used in IPA). Then  IPAEntrySimpleLDAPObject would subclass
> IPASimpleLDAPObject which knows about Entry objects.
>
> The reason for the suggested class hierarchy is because DN objects will be
> used whenever we talk to LDAP (in the future we may want to add other IPA
> specific classes which will always be used). We don't add Entry support to
> the the IPASimpleLDAPObject class because Entry objects are (currently)
> only used in IPAdmin.
>
> What this patch does is:
>
> * Introduce IPASimpleLDAPObject derived from
>    SimpleLDAPObject. IPASimpleLDAPObject is DN object aware.
>
> * Introduce IPAEntryLDAPObject derived from
>    IPASimpleLDAPObject. IPAEntryLDAPObject is Entry object aware.
>
> * Derive IPAdmin from IPAEntryLDAPObject and remove the funky method
>    wrapping from IPAdmin.
>
> * Code which called add_s() with an Entry or Entity object now calls
>    addEntry(). addEntry() always existed, it just wasn't always
>    used. add_s() had been modified to accept Entry or Entity object
>    (why didn't we just call addEntry()?). The add*() ldap routine in
>    IPAEntryLDAPObject have been subclassed to accept Entry and Entity
>    objects, but that should proably be removed in the future and just
>    use addEntry().
>
> * Replace the call to ldap.initialize() in ldap2.create_connection()
>    with a class constructor for IPASimpleLDAPObject. The
>    ldap.initialize() is a convenience function in python-ldap, but it
>    always returns a SimpleLDAPObject created via the SimpleLDAPObject
>    constructor, thus ldap.initialize() did not allow subclassing, yet
>    has no particular ease-of-use advantage thus we better off using the
>    obvious class constructor mechanism.
>
> * Fix the use of _handle_errors(), it's not necessary to construct an
>    empty dict to pass to it.
>
> If we follow the standard class derivation pattern for ldap we can make us
> of our own ldap utilities in a far easier, cleaner and more efficient
> manner.
ack
> --
> John Dennis<jdennis at redhat.com>
>
> Looking to carve out IT costs?
> www.redhat.com/carveoutcosts/
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20110929/c571aaff/attachment.htm>


More information about the Freeipa-devel mailing list