Embed origiginal values inside user, and have update_user pass in a user

object.  Based on rcrit's original patch.
Push scalar to list value conversion inside funcs.py.

5 files changed, 65 insertions, 55 deletions

diff --git a/ipa-admintools/ipa-usermod b/ipa-admintools/ipa-usermod
index cb221e446..bcd06821d 100644
--- a/ipa-admintools/ipa-usermod
+++ b/ipa-admintools/ipa-usermod
@@ -48,27 +48,23 @@ def parse_options():
     return options, args
 
 def main():
-    olduser={}
-    newuser={}
     options, args = parse_options()
 
     if len(args) != 2:
         usage()
 
     client = ipaclient.IPAClient()
-    u = client.get_user(args[1])
-    olduser = u.toDict()
-    newuser = u.toDict()
+    user = client.get_user(args[1])
 
     if options.gecos:
-        newuser['gecos'] = [options.gecos]
+        user.setValue('gecos', options.gecos)
     if options.directory:
-        newuser['homedirectory'] = [options.directory]
+        user.setValue('homedirectory', options.directory)
     if options.shell:
-        newuser['loginshell'] = [options.shell]
+        user.setValue('loginshell', options.shell)
 
     try:
-        client.update_user(olduser, newuser)
+        client.update_user(user)
         print args[1] + " successfully modified"
     except xmlrpclib.Fault, f:
         print f.faultString
diff --git a/ipa-python/ipaclient.py b/ipa-python/ipaclient.py
index 2d4e727ae..729189997 100644
--- a/ipa-python/ipaclient.py
+++ b/ipa-python/ipaclient.py
@@ -116,14 +116,12 @@ class IPAClient:
 
         return users
 
-    def update_user(self,olduser,newuser):
-        """Update a user entry. olduser is a dict of attribute/value pairs
-           of the original entry. newuser is a dict of attribute/value pairs
-           of the new entry."""
+    def update_user(self,user):
+        """Update a user entry."""
 
         realm = config.config.get_realm()
 
-        result = self.transport.update_user(olduser,newuser)
+        result = self.transport.update_user(user.origDataDict(), user.toDict())
         return result
 
     def mark_user_deleted(self,uid):
diff --git a/ipa-python/user.py b/ipa-python/user.py
index ecbd2845c..38a634725 100644
--- a/ipa-python/user.py
+++ b/ipa-python/user.py
@@ -11,7 +11,8 @@ class User:
     In python-ldap, entries are returned as a list of 2-tuples.
     Instance variables:
     dn - string - the string DN of the entry
-    data - cidict - case insensitive dict of the attributes and values"""
+    data - cidict - case insensitive dict of the attributes and values
+    orig_data - cidict - case insentiive dict of the original attributes and values"""
 
     def __init__(self,entrydata):
         """data is the raw data returned from the python-ldap result method,
@@ -32,6 +33,8 @@ class User:
             self.dn = ''
             self.data = ldap.cidict.cidict()
 
+        self.orig_data = dict(self.data)
+
     def __nonzero__(self):
         """This allows us to do tests like if entry: returns false if there is no data,
         true otherwise"""
@@ -41,6 +44,14 @@ class User:
         """Return True if this entry has an attribute named name, False otherwise"""
         return self.data and self.data.has_key(name)
 
+    def __setattr__(self,name,value):
+        """One should use setValue() or setValues() to set values except for
+           dn and data which are special."""
+        if name != 'dn' and name != 'data' and name != 'orig_data':
+            raise KeyError, 'use setValue() or setValues()'
+        else:
+            self.__dict__[name] = value
+
     def __getattr__(self,name):
         """If name is the name of an LDAP attribute, return the first value for that
         attribute - equivalent to getValue - this allows the use of
@@ -72,9 +83,9 @@ class User:
            ent.setValue('name', ('value1', 'value2', ..., 'valueN'))
         Since *value is a tuple, we may have to extract a list or tuple from that
         tuple as in the last two examples above"""
-        if (len(value[0]) < 1):
+        if (len(value) < 1):
             return
-        if isinstance(value[0],list) or isinstance(value[0],tuple):
+        if (len(value) == 1):
             self.data[name] = value[0]
         else:
             self.data[name] = value
@@ -100,6 +111,14 @@ class User:
         """Return a list of all attributes in the entry"""
         return self.data.keys()
 
+    def origDataDict(self):
+        """Returns a dict of the original values of the user.  Used for updates."""
+        result = {}
+        for k in self.orig_data.keys():
+            result[k] = self.orig_data[k]
+        result['dn'] = self.dn
+        return result
+
 #    def __str__(self):
 #        """Convert the Entry to its LDIF representation"""
 #        return self.__repr__()
diff --git a/ipa-server/ipa-gui/ipagui/controllers.py b/ipa-server/ipa-gui/ipagui/controllers.py
index 36c1bf84b..b285685f5 100644
--- a/ipa-server/ipa-gui/ipagui/controllers.py
+++ b/ipa-server/ipa-gui/ipagui/controllers.py
@@ -32,25 +32,10 @@ def restrict_post():
         turbogears.flash("This method only accepts posts")
         raise turbogears.redirect("/")
 
-def to_ldap_hash(orig):
-    """LDAP hashes expect all values to be a list.  This method converts single
-       entries to a list."""
-    new={}
-    for (k,v) in orig.iteritems():
-        if v == None:
-            continue
-        if not isinstance(v, list) and k != 'dn':
-            v = [v]
-        new[k] = v
-
-    return new
-
-def set_ldap_value(hash, key, value):
-    """Converts unicode strings to normal strings
-       (because LDAP is choking on unicode strings"""
+def utf8_encode(value):
     if value != None:
         value = value.encode('utf-8')
-    hash[key] = value
+    return value
 
 
 class Root(controllers.RootController):
@@ -86,11 +71,11 @@ class Root(controllers.RootController):
 
         try:
             new_user = {}
-            set_ldap_value(new_user, 'uid', kw.get('uid'))
-            set_ldap_value(new_user, 'givenname', kw.get('givenname'))
-            set_ldap_value(new_user, 'sn', kw.get('sn'))
-            set_ldap_value(new_user, 'mail', kw.get('mail'))
-            set_ldap_value(new_user, 'telephonenumber', kw.get('telephonenumber'))
+            new_user['uid'] = utf8_encode(kw.get('uid'))
+            new_user['givenname'] = utf8_encode(kw.get('givenname'))
+            new_user['sn'] = utf8_encode(kw.get('sn'))
+            new_user['mail'] = utf8_encode(kw.get('mail'))
+            new_user['telephonenumber'] = utf8_encode(kw.get('telephonenumber'))
 
             rv = client.add_user(new_user)
             turbogears.flash("%s added!" % kw['uid'])
@@ -107,11 +92,11 @@ class Root(controllers.RootController):
             turbogears.flash("There was a problem with the form!")
 
         user = client.get_user(uid)
-        user_hash = user.toDict()
+        user_dict = user.toDict()
         # store a copy of the original user for the update later
-        user_data = b64encode(dumps(user_hash))
-        user_hash['user_orig'] = user_data
-        return dict(form=user_edit_form, user=user_hash)
+        user_data = b64encode(dumps(user_dict))
+        user_dict['user_orig'] = user_data
+        return dict(form=user_edit_form, user=user_dict)
 
     @expose()
     def userupdate(self, **kw):
@@ -127,23 +112,21 @@ class Root(controllers.RootController):
                         tg_template='ipagui.templates.useredit')
 
         try:
-            orig_user = loads(b64decode(kw.get('user_orig')))
+            orig_user_dict = loads(b64decode(kw.get('user_orig')))
 
-            new_user = dict(orig_user)
-            set_ldap_value(new_user, 'givenname', kw.get('givenname'))
-            set_ldap_value(new_user, 'sn', kw.get('sn'))
-            set_ldap_value(new_user, 'mail', kw.get('mail'))
-            set_ldap_value(new_user, 'telephonenumber', kw.get('telephonenumber'))
+            new_user = ipa.user.User(orig_user_dict)
+            new_user.setValue('givenname', utf8_encode(kw.get('givenname')))
+            new_user.setValue('sn', utf8_encode(kw.get('sn')))
+            new_user.setValue('mail', utf8_encode(kw.get('mail')))
+            new_user.setValue('telephonenumber', utf8_encode(kw.get('telephonenumber')))
             #
             # this is a hack until we decide on the policy for names/cn/sn/givenName
             #
-            set_ldap_value(new_user, 'cn', 
-                           "%s %s" % (kw.get('givenname'), kw.get('sn')))
+            new_user.setValue('cn',
+                           "%s %s" % (new_user.getValue('givenname'),
+                                      new_user.getValue('sn')))
 
-            orig_user = to_ldap_hash(orig_user)
-            new_user = to_ldap_hash(new_user)
-
-            rv = client.update_user(orig_user, new_user)
+            rv = client.update_user(new_user)
             turbogears.flash("%s updated!" % kw['uid'])
             raise turbogears.redirect('/usershow', uid=kw['uid'])
         except xmlrpclib.Fault, f:
diff --git a/ipa-server/xmlrpc-server/funcs.py b/ipa-server/xmlrpc-server/funcs.py
index a182d4b26..6f7e4e66b 100644
--- a/ipa-server/xmlrpc-server/funcs.py
+++ b/ipa-server/xmlrpc-server/funcs.py
@@ -363,6 +363,17 @@ class IPAServer:
     
         return users
 
+    def convert_scalar_values(self, orig_dict):
+        """LDAP update dicts expect all values to be a list (except for dn).
+           This method converts single entries to a list."""
+        new_dict={}
+        for (k,v) in orig_dict.iteritems():
+            if not isinstance(v, list) and k != 'dn':
+                v = [v]
+            new_dict[k] = v
+
+        return new_dict
+
     def update_user (self, args, newuser=None, opts=None):
         """Update a user in LDAP"""
         global _LDAPPool
@@ -384,6 +395,9 @@ class IPAServer:
         if (isinstance(newuser, tuple)):
             newuser = newuser[0]
 
+        olduser = self.convert_scalar_values(olduser)
+        newuser = self.convert_scalar_values(newuser)
+
         # Should be able to get this from either the old or new user
         # but just in case someone has decided to try changing it, use the
         # original