21a1414 ticket #1870 - subclass SimpleLDAPObject

2 files Authored by jdennis 12 years ago, Committed by simo 12 years ago,
    ticket #1870 - subclass SimpleLDAPObject
    
    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.
    
        
file modified
+47 -58
file modified
+114 -10