#1870 We should support sublclassing python ldap LDAP*Object to support customization for IPA usage
Closed: Fixed None Opened 12 years ago by jdennis.

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 objects being passed into and returned from ldap operations.

I suggest we introduce an IPASimpleLDAPObject class which knows about DN objects and an IPAEntrySimpleLDAPObject class which knows about Entry objects, it would be derived from IPASimpleLDAPOjbect class.

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 (why, they're pretty useful). Then:

  • Derive IPAdmin from IPAEntryLDAPObject.

  • 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 it does not allow subclassing, yet has no particular ease-of-use advantage.

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.


patch submitted as:
[PATCH 49/49] ticket #1870 - subclass SimpleLDAPObject

Moving to 3.0 bucket as there is not enough resources to test these core changes properly in 2.1 branch.

Moving the ticket to the next month iteration.

Metadata Update from @jdennis:
- Issue assigned to jdennis
- Issue set to the milestone: FreeIPA 3.0 Core Effort - 2011/11

7 years ago

Login to comment on this ticket.

Metadata