Hi,

these patches add the ability to access and manipulate raw attribute values as they are returned from python-ldap to the LDAPEntry class. This is useful for comparing entries, computing modlists for the modify operation, deleting values that don't match the syntax of an attribute, etc., because we don't need to care what syntax does a particular attribute use, or what Python type is used for it in the framework (raw values are always list of str). It should also make interaction with non-389 DS LDAP servers easier in the framework.

(It might be too late for this kind of changes to get into 3.2 now, I'm posting these patches mainly so that you are aware that they exist.)

Honza

--
Jan Cholasta
>From b365ef78e5f784661261cba1c51f24703d5a3437 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Tue, 26 Feb 2013 11:27:55 +0100
Subject: [PATCH 1/8] Make LDAPEntry a wrapper around dict rather than a dict
 subclass.

---
 ipaserver/ipaldap.py | 100 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 36 deletions(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index bdc2d44..da9a60d 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -582,8 +582,8 @@ class IPASimpleLDAPObject(object):
 # r = result[0]
 # r[0] == r.dn
 # r[1] == r.data
-class LDAPEntry(dict):
-    __slots__ = ('_conn', '_dn', '_names', '_orig')
+class LDAPEntry(object):
+    __slots__ = ('_conn', '_dn', '_names', '_data', '_orig')
 
     def __init__(self, _conn, _dn=None, _obj=None, **kwargs):
         """
@@ -601,8 +601,6 @@ class LDAPEntry(dict):
 
         Keyword arguments can be used to override values of specific attributes.
         """
-        super(LDAPEntry, self).__init__()
-
         if isinstance(_conn, LDAPEntry):
             assert _dn is None
             _dn = _conn
@@ -611,13 +609,15 @@ class LDAPEntry(dict):
         if isinstance(_dn, LDAPEntry):
             assert _obj is None
             _obj = _dn
-            _dn = DN(_dn._dn)
+            _dn = _dn._dn
 
         if isinstance(_obj, LDAPEntry):
+            data = dict(_obj._data)
             orig = _obj._orig
         else:
             if _obj is None:
                 _obj = {}
+            data = {}
             orig = self
 
         assert isinstance(_conn, IPASimpleLDAPObject)
@@ -625,8 +625,9 @@ class LDAPEntry(dict):
 
         self._conn = _conn
         self._dn = _dn
-        self._orig = orig
         self._names = CIDict()
+        self._data = data
+        self._orig = orig
 
         self.update(_obj, **kwargs)
 
@@ -655,8 +656,7 @@ class LDAPEntry(dict):
         return self._orig
 
     def __repr__(self):
-        return '%s(%r, %s)' % (type(self).__name__, self._dn,
-            super(LDAPEntry, self).__repr__())
+        return '%s(%r, %r)' % (type(self).__name__, self._dn, self._data)
 
     def copy(self):
         return LDAPEntry(self)
@@ -664,11 +664,8 @@ class LDAPEntry(dict):
     def clone(self):
         result = LDAPEntry(self._conn, self._dn)
 
-        for name in self.iterkeys():
-            super(LDAPEntry, result).__setitem__(
-                name, deepcopy(super(LDAPEntry, self).__getitem__(name)))
-
         result._names = deepcopy(self._names)
+        result._data = deepcopy(self._data)
         if self._orig is not self:
             result._orig = self._orig.clone()
 
@@ -704,7 +701,7 @@ class LDAPEntry(dict):
                     if keyname == oldname:
                         self._names[altname] = name
 
-                super(LDAPEntry, self).__delitem__(oldname)
+                del self._data[oldname]
         else:
             self._names[name] = name
 
@@ -720,7 +717,7 @@ class LDAPEntry(dict):
                         altname = altname.decode('utf-8')
                         self._names[altname] = name
 
-        super(LDAPEntry, self).__setitem__(name, value)
+        self._data[name] = value
 
     def setdefault(self, name, default):
         if name not in self:
@@ -737,6 +734,9 @@ class LDAPEntry(dict):
         name = self._names[name]
         return name
 
+    def _get(self, name):
+        return self._data[self._get_attr_name(name)]
+
     def __getitem__(self, name):
         # for python-ldap tuple compatibility
         if name == 0:
@@ -744,16 +744,14 @@ class LDAPEntry(dict):
         elif name == 1:
             return self
 
-        return super(LDAPEntry, self).__getitem__(self._get_attr_name(name))
+        return self._get(name)
 
     def get(self, name, default=None):
         try:
-            name = self._get_attr_name(name)
+            return self._get(name)
         except KeyError:
             return default
 
-        return super(LDAPEntry, self).get(name, default)
-
     def single_value(self, name, default=_missing):
         """Return a single attribute value
 
@@ -763,8 +761,7 @@ class LDAPEntry(dict):
         If the entry is missing and default is not given, raise KeyError.
         """
         try:
-            attr_name = self._get_attr_name(name)
-            values = super(LDAPEntry, self).__getitem__(attr_name)
+            values = self._get(name)
         except KeyError:
             if default is _missing:
                 raise
@@ -776,41 +773,72 @@ class LDAPEntry(dict):
                 '%s has %s values, one expected' % (name, len(values)))
         return values[0]
 
-    def _del_attr_name(self, name):
+    def __delitem__(self, name):
         name = self._get_attr_name(name)
 
         for (altname, keyname) in self._names.items():
             if keyname == name:
                 del self._names[altname]
 
-        return name
-
-    def __delitem__(self, name):
-        super(LDAPEntry, self).__delitem__(self._del_attr_name(name))
+        del self._data[name]
 
-    def pop(self, name, *default):
+    def pop(self, name, default=_missing):
         try:
-            name = self._del_attr_name(name)
+            value = self._get(name)
+            del self[name]
+            return value
         except KeyError:
-            if not default:
+            if default is _missing:
                 raise
-
-        return super(LDAPEntry, self).pop(name, *default)
+            return default
 
     def popitem(self):
-        name, value = super(LDAPEntry, self).popitem()
-        self._del_attr_name(name)
-        return (name, value)
+        try:
+            name = self.iterkeys().next()
+        except StopIteration:
+            # raise dict's popitem KeyError
+            {}.popitem()
+
+        return (name, self.pop(name))
 
     def clear(self):
-        super(LDAPEntry, self).clear()
         self._names.clear()
+        self._data.clear()
+
+    def __len__(self):
+        return len(self._data)
 
     def __contains__(self, name):
         return self._names.has_key(self._attr_name(name))
 
-    def has_key(self, name):
-        return name in self
+    has_key = __contains__
+
+    def __cmp__(self, other):
+        if not isinstance(other, LDAPEntry):
+            return False
+        if self._dn != other._dn:
+            return False
+        return self._data == other._data
+
+    def iterkeys(self):
+        return self._data.iterkeys()
+
+    def keys(self):
+        return list(self.iterkeys())
+
+    def itervalues(self):
+        for name in self.iterkeys():
+            yield self._get(name)
+
+    def values(self):
+        return list(self.itervalues())
+
+    def iteritems(self):
+        for name in self.iterkeys():
+            yield (name, self._get(name))
+
+    def items(self):
+        return list(self.iteritems())
 
     # for python-ldap tuple compatibility
     def __iter__(self):
-- 
1.8.1

>From 878fc1c2b2dd1ec6d19b92a6ef5158174154bca8 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Tue, 26 Feb 2013 11:31:56 +0100
Subject: [PATCH 2/8] Introduce IPASimpleLDAPObject.decode method for decoding
 LDAP values.

This method replaces IPASimpleLDAPObject.convert_value_list.
---
 ipaserver/ipaldap.py | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index da9a60d..5c99b1c 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -359,26 +359,30 @@ class IPASimpleLDAPObject(object):
         else:
             raise TypeError("attempt to pass unsupported type to ldap, value=%s type=%s" %(val, type(val)))
 
-    def convert_value_list(self, attr, target_type, values):
+    def decode(self, val, attr):
         '''
         '''
-
-        ipa_values = []
-
-        for original_value in values:
-            if isinstance(target_type, type) and isinstance(original_value, target_type):
-                ipa_value = original_value
-            else:
-                try:
-                    ipa_value = target_type(original_value)
-                except Exception, e:
-                    msg = 'unable to convert the attribute "%s" value "%s" to type %s' % (attr, original_value, target_type)
-                    self.log.error(msg)
-                    raise ValueError(msg)
-
-            ipa_values.append(ipa_value)
-
-        return ipa_values
+        if isinstance(val, str):
+            target_type = self._SYNTAX_MAPPING.get(self.get_syntax(attr), unicode_from_utf8)
+            if target_type is str:
+                return val
+            try:
+                return target_type(val)
+            except Exception, e:
+                msg = 'unable to convert the attribute "%s" value "%s" to type %s' % (attr, val, target_type)
+                self.log.error(msg)
+                raise ValueError(msg)
+        elif isinstance(val, list):
+            return [self.decode(m, attr) for m in val]
+        elif isinstance(val, tuple):
+            return tuple(self.decode(m, attr) for m in val)
+        elif isinstance(val, dict):
+            dct = dict((unicode_from_utf8(k), self.decode(v, k)) for k, v in val.iteritems())
+            return dct
+        elif val is None:
+            return None
+        else:
+            raise TypeError("attempt to pass unsupported type from ldap, value=%s type=%s" %(val, type(val)))
 
     def convert_result(self, result):
         '''
@@ -406,8 +410,7 @@ class IPASimpleLDAPObject(object):
             ipa_entry = LDAPEntry(self, DN(original_dn))
 
             for attr, original_values in original_attrs.items():
-                target_type = self._SYNTAX_MAPPING.get(self.get_syntax(attr), unicode_from_utf8)
-                ipa_entry[attr] = self.convert_value_list(attr, target_type, original_values)
+                ipa_entry[attr] = self.decode(original_values, attr)
 
             ipa_result.append(ipa_entry)
 
-- 
1.8.1

>From cbff921d8c939034521b344e0395a654f46660f3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Tue, 26 Feb 2013 11:49:01 +0100
Subject: [PATCH 3/8] Always use lists for values in LDAPEntry internally.

Outside of LDAPEntry, it is still possible to use non-lists. Once we enforce
lists for attribute values, this will be removed.
---
 ipaserver/ipaldap.py | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 5c99b1c..e2381bf 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -586,7 +586,7 @@ class IPASimpleLDAPObject(object):
 # r[0] == r.dn
 # r[1] == r.data
 class LDAPEntry(object):
-    __slots__ = ('_conn', '_dn', '_names', '_data', '_orig')
+    __slots__ = ('_conn', '_dn', '_names', '_data', '_not_list', '_orig')
 
     def __init__(self, _conn, _dn=None, _obj=None, **kwargs):
         """
@@ -616,11 +616,13 @@ class LDAPEntry(object):
 
         if isinstance(_obj, LDAPEntry):
             data = dict(_obj._data)
+            not_list = set(_obj._not_list)
             orig = _obj._orig
         else:
             if _obj is None:
                 _obj = {}
             data = {}
+            not_list = set()
             orig = self
 
         assert isinstance(_conn, IPASimpleLDAPObject)
@@ -630,6 +632,7 @@ class LDAPEntry(object):
         self._dn = _dn
         self._names = CIDict()
         self._data = data
+        self._not_list = not_list
         self._orig = orig
 
         self.update(_obj, **kwargs)
@@ -669,6 +672,7 @@ class LDAPEntry(object):
 
         result._names = deepcopy(self._names)
         result._data = deepcopy(self._data)
+        result._not_list = deepcopy(self._not_list)
         if self._orig is not self:
             result._orig = self._orig.clone()
 
@@ -705,6 +709,7 @@ class LDAPEntry(object):
                         self._names[altname] = name
 
                 del self._data[oldname]
+                self._not_list.discard(oldname)
         else:
             self._names[name] = name
 
@@ -720,6 +725,13 @@ class LDAPEntry(object):
                         altname = altname.decode('utf-8')
                         self._names[altname] = name
 
+        if not isinstance(value, list):
+            if value is None:
+                value = []
+            else:
+                value = [value]
+            self._not_list.add(name)
+
         self._data[name] = value
 
     def setdefault(self, name, default):
@@ -738,7 +750,19 @@ class LDAPEntry(object):
         return name
 
     def _get(self, name):
-        return self._data[self._get_attr_name(name)]
+        name = self._get_attr_name(name)
+
+        value = self._data[name]
+        assert isinstance(value, list)
+
+        if name in self._not_list:
+            assert len(value) <= 1
+            if value:
+                value = value[0]
+            else:
+                value = None
+
+        return value
 
     def __getitem__(self, name):
         # for python-ldap tuple compatibility
@@ -784,6 +808,7 @@ class LDAPEntry(object):
                 del self._names[altname]
 
         del self._data[name]
+        self._not_list.discard(name)
 
     def pop(self, name, default=_missing):
         try:
@@ -807,6 +832,7 @@ class LDAPEntry(object):
     def clear(self):
         self._names.clear()
         self._data.clear()
+        self._not_list.clear()
 
     def __len__(self):
         return len(self._data)
-- 
1.8.1

>From 689f107942e5f44ded5387f7600412eb12359a3c Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Wed, 27 Feb 2013 09:16:49 +0100
Subject: [PATCH 4/8] Store both encoded and decoded values in LDAPEntry.

Each set of values is accessible via different methods of LDAPEntry. Changes
in one set are synchronized to the other when an attribute is accessed.
---
 ipalib/plugins/baseldap.py        |   2 +
 ipaserver/ipaldap.py              | 209 ++++++++++++++++++++++++++++++++------
 tests/test_ipaserver/test_ldap.py |  48 +++++++++
 3 files changed, 228 insertions(+), 31 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 03ff7f9..716cf0c 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -917,6 +917,8 @@ last, after all sets and adds."""),
                     raise errors.ValidationError(name=attr, error=err.error)
                 except errors.ConversionError, err:
                     raise errors.ConversionError(name=attr, error=err.error)
+                if isinstance(value, tuple):
+                    value = list(value)
                 entry_attrs[attr] = value
             else:
                 # unknown attribute: remove duplicite and invalid values
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index e2381bf..2a7b9dc 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -586,7 +586,8 @@ class IPASimpleLDAPObject(object):
 # r[0] == r.dn
 # r[1] == r.data
 class LDAPEntry(object):
-    __slots__ = ('_conn', '_dn', '_names', '_data', '_not_list', '_orig')
+    __slots__ = ('_conn', '_dn', '_names', '_nice', '_raw', '_synced',
+                 '_not_list', '_orig')
 
     def __init__(self, _conn, _dn=None, _obj=None, **kwargs):
         """
@@ -615,13 +616,17 @@ class LDAPEntry(object):
             _dn = _dn._dn
 
         if isinstance(_obj, LDAPEntry):
-            data = dict(_obj._data)
+            nice = dict(_obj._nice)
+            raw = dict(_obj._raw)
+            synced = dict(_obj._synced)
             not_list = set(_obj._not_list)
             orig = _obj._orig
         else:
             if _obj is None:
                 _obj = {}
-            data = {}
+            nice = {}
+            raw = {}
+            synced = {}
             not_list = set()
             orig = self
 
@@ -631,7 +636,9 @@ class LDAPEntry(object):
         self._conn = _conn
         self._dn = _dn
         self._names = CIDict()
-        self._data = data
+        self._nice = nice
+        self._raw = raw
+        self._synced = synced
         self._not_list = not_list
         self._orig = orig
 
@@ -662,7 +669,7 @@ class LDAPEntry(object):
         return self._orig
 
     def __repr__(self):
-        return '%s(%r, %r)' % (type(self).__name__, self._dn, self._data)
+        return '%s(%r, %r)' % (type(self).__name__, self._dn, self._raw)
 
     def copy(self):
         return LDAPEntry(self)
@@ -671,7 +678,9 @@ class LDAPEntry(object):
         result = LDAPEntry(self._conn, self._dn)
 
         result._names = deepcopy(self._names)
-        result._data = deepcopy(self._data)
+        result._nice = deepcopy(self._nice)
+        result._raw = deepcopy(self._raw)
+        result._synced = deepcopy(self._synced)
         result._not_list = deepcopy(self._not_list)
         if self._orig is not self:
             result._orig = self._orig.clone()
@@ -697,9 +706,7 @@ class LDAPEntry(object):
 
         return name
 
-    def __setitem__(self, name, value):
-        name = self._attr_name(name)
-
+    def _add_attr_name(self, name):
         if self._names.has_key(name):
             oldname = self._names[name]
 
@@ -708,8 +715,13 @@ class LDAPEntry(object):
                     if keyname == oldname:
                         self._names[altname] = name
 
-                del self._data[oldname]
-                self._not_list.discard(oldname)
+                self._nice[name] = self._nice.pop(oldname)
+                self._raw[name] = self._raw.pop(oldname)
+                if oldname in self._synced:
+                    self._synced[name] = self._synced.pop(oldname)
+                if oldname in self._not_list:
+                    self._not_list.remove(oldname)
+                    self._not_list.add(name)
         else:
             self._names[name] = name
 
@@ -725,14 +737,49 @@ class LDAPEntry(object):
                         altname = altname.decode('utf-8')
                         self._names[altname] = name
 
+    def set_nice(self, name, value):
+        name = self._attr_name(name)
+        self._add_attr_name(name)
+
         if not isinstance(value, list):
             if value is None:
                 value = []
             else:
                 value = [value]
             self._not_list.add(name)
+        else:
+            self._not_list.discard(name)
+
+        self._nice[name] = value
+        self._synced.pop(name, None)
 
-        self._data[name] = value
+        raw = self._raw.setdefault(name, None)
+        if raw is not None:
+            raw[:] = []
+            self._sync(name)
+
+    def set_raw(self, name, value):
+        name = self._attr_name(name)
+
+        if not isinstance(value, list):
+            raise TypeError("%s value must be list, got %s object %r" % (
+                name, value.__class__.__name__, value))
+        for (i, item) in enumerate(value):
+            if not isinstance(item, str):
+                raise TypeError("%s[%d] value must be str, got %s object %r" % (
+                    name, i, item.__class__.__name__, item))
+
+        self._add_attr_name(name)
+
+        self._raw[name] = value
+        self._synced.pop(name, None)
+
+        nice = self._nice.setdefault(name, None)
+        if nice is not None:
+            nice[:] = []
+            self._sync(name)
+
+    __setitem__ = set_nice
 
     def setdefault(self, name, default):
         if name not in self:
@@ -740,6 +787,12 @@ class LDAPEntry(object):
         return self[name]
 
     def update(self, _obj={}, **kwargs):
+        if isinstance(_obj, LDAPEntry):
+            for name in _obj.iterkeys():
+                value = _obj.get_raw(name)  #pylint: disable=E1103
+                self.set_raw(name, value)
+            _obj = {}
+
         _obj = dict(_obj, **kwargs)
         for (name, value) in _obj.iteritems():
             self[name] = value
@@ -749,10 +802,60 @@ class LDAPEntry(object):
         name = self._names[name]
         return name
 
+    def _sync(self, name):
+        nice = self._nice[name]
+        if nice is None:
+            nice = self._nice[name] = []
+
+        raw = self._raw[name]
+        if raw is None:
+            raw = self._raw[name] = []
+
+        nice_synced, raw_synced = self._synced.setdefault(name, ([], []))
+        if nice == nice_synced and raw == raw_synced:
+            return
+
+        nice_adds = set(nice) - set(nice_synced)
+        nice_dels = set(nice_synced) - set(nice)
+        raw_adds = set(raw) - set(raw_synced)
+        raw_dels = set(raw_synced) - set(raw)
+
+        for value in nice_dels:
+            value = self._conn.encode(value)
+            if value in raw_adds:
+                continue
+            raw.remove(value)
+
+        for value in raw_dels:
+            value = self._conn.decode(value, name)
+            if value in nice_adds:
+                continue
+            nice.remove(value)
+
+        for value in nice_adds:
+            value = self._conn.encode(value)
+            if value in raw_dels:
+                continue
+            raw.append(value)
+
+        for value in raw_adds:
+            value = self._conn.decode(value, name)
+            if value in nice_dels:
+                continue
+            nice.append(value)
+
+        self._synced[name] = (deepcopy(nice), deepcopy(raw))
+
+        if len(nice) > 1:
+            self._not_list.discard(name)
+
     def _get(self, name):
         name = self._get_attr_name(name)
 
-        value = self._data[name]
+        if self._raw[name] is not None:
+            self._sync(name)
+
+        value = self._nice[name]
         assert isinstance(value, list)
 
         if name in self._not_list:
@@ -764,6 +867,28 @@ class LDAPEntry(object):
 
         return value
 
+    def get_nice(self, name, default=None):
+        try:
+            return self._get(name)
+        except KeyError:
+            return default
+
+    def get_raw(self, name, default=_missing):
+        try:
+            name = self._get_attr_name(name)
+        except KeyError:
+            if default is _missing:
+                return []
+            return default
+
+        if self._nice[name] is not None:
+            self._sync(name)
+
+        value = self._raw[name]
+        assert isinstance(value, list)
+
+        return value
+
     def __getitem__(self, name):
         # for python-ldap tuple compatibility
         if name == 0:
@@ -773,11 +898,7 @@ class LDAPEntry(object):
 
         return self._get(name)
 
-    def get(self, name, default=None):
-        try:
-            return self._get(name)
-        except KeyError:
-            return default
+    get = get_nice
 
     def single_value(self, name, default=_missing):
         """Return a single attribute value
@@ -807,7 +928,9 @@ class LDAPEntry(object):
             if keyname == name:
                 del self._names[altname]
 
-        del self._data[name]
+        del self._nice[name]
+        del self._raw[name]
+        self._synced.pop(name, None)
         self._not_list.discard(name)
 
     def pop(self, name, default=_missing):
@@ -831,11 +954,13 @@ class LDAPEntry(object):
 
     def clear(self):
         self._names.clear()
-        self._data.clear()
+        self._nice.clear()
+        self._raw.clear()
+        self._synced.clear()
         self._not_list.clear()
 
     def __len__(self):
-        return len(self._data)
+        return len(self._nice)
 
     def __contains__(self, name):
         return self._names.has_key(self._attr_name(name))
@@ -847,27 +972,49 @@ class LDAPEntry(object):
             return False
         if self._dn != other._dn:
             return False
-        return self._data == other._data
+        return dict(self.iteritems_raw()) == dict(other.iteritems_raw())
 
     def iterkeys(self):
-        return self._data.iterkeys()
+        return self._nice.iterkeys()
 
     def keys(self):
         return list(self.iterkeys())
 
-    def itervalues(self):
+    def itervalues_nice(self):
+        for name in self.iterkeys():
+            yield self.get_nice(name)
+
+    def itervalues_raw(self):
         for name in self.iterkeys():
-            yield self._get(name)
+            yield self.get_raw(name)
+
+    itervalues = itervalues_nice
 
-    def values(self):
-        return list(self.itervalues())
+    def values_nice(self):
+        return list(self.itervalues_nice())
 
-    def iteritems(self):
+    def values_raw(self):
+        return list(self.itervalues_raw())
+
+    values = values_nice
+
+    def iteritems_nice(self):
+        for name in self.iterkeys():
+            yield (name, self.get_nice(name))
+
+    def iteritems_raw(self):
         for name in self.iterkeys():
-            yield (name, self._get(name))
+            yield (name, self.get_raw(name))
+
+    iteritems = iteritems_nice
+
+    def items_nice(self):
+        return list(self.iteritems_nice())
+
+    def items_raw(self):
+        return list(self.iteritems_raw())
 
-    def items(self):
-        return list(self.iteritems())
+    items = items_nice
 
     # for python-ldap tuple compatibility
     def __iter__(self):
diff --git a/tests/test_ipaserver/test_ldap.py b/tests/test_ipaserver/test_ldap.py
index 738dba7..da0a9c0 100644
--- a/tests/test_ipaserver/test_ldap.py
+++ b/tests/test_ipaserver/test_ldap.py
@@ -254,3 +254,51 @@ class test_LDAPEntry(object):
         assert e.single_value('commonname') == self.cn1[0]
         assert e.single_value('COMMONNAME', 'default') == self.cn1[0]
         assert e.single_value('bad key', 'default') == 'default'
+
+    def test_sync(self):
+        e = self.entry
+
+        nice = e['test'] = [1, 2, 3]
+        assert e['test'] is nice
+
+        raw = e.get_raw('test')
+        assert raw == ['1', '2', '3']
+
+        nice.remove(1)
+        assert e.get_raw('test') is raw
+        assert raw == ['2', '3']
+
+        raw.append('4')
+        assert e['test'] is nice
+        assert nice == [2, 3, u'4']
+
+        nice.remove(2)
+        raw.append('5')
+        assert nice == [3, u'4']
+        assert raw == ['2', '3', '4', '5']
+        assert e['test'] is nice
+        assert e.get_raw('test') is raw
+        assert nice == [3, u'4', u'5']
+        assert raw == ['3', '4', '5']
+
+        nice.insert(0, 2)
+        raw.remove('4')
+        assert nice == [2, 3, u'4', u'5']
+        assert raw == ['3', '5']
+        assert e.get_raw('test') is raw
+        assert e['test'] is nice
+        assert nice == [2, 3, u'5']
+        assert raw == ['3', '5', '2']
+
+        raw = ['a', 'b']
+        e.set_raw('test', raw)
+        assert e['test'] is nice
+        assert nice == [u'a', u'b']
+
+        nice = 'not list'
+        e['test'] = nice
+        assert e['test'] == nice
+        assert raw == ['not list']
+
+        raw.append('second')
+        assert e['test'] == ['not list', u'second']
-- 
1.8.1

>From 616f45d21c4fbc92f5423f6a553cbcd186f08887 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Mon, 11 Feb 2013 16:26:58 +0100
Subject: [PATCH 5/8] Make sure attributeTypes updates are done before
 objectClasses updates.

---
 ipaserver/ipaldap.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 2a7b9dc..70f0ec0 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1969,7 +1969,10 @@ class IPAdmin(LDAPClient):
             # replace any existing schema.
             if old_entry.get('dn', DN()) == DN(('cn', 'schema')):
                 if len(adds) > 0:
-                    modlist.append((ldap.MOD_ADD, key, adds))
+                    if key == 'attributetypes':
+                        modlist.insert(0, (ldap.MOD_ADD, key, adds))
+                    else:
+                        modlist.append((ldap.MOD_ADD, key, adds))
             else:
                 if adds:
                     if force_replace:
-- 
1.8.1

>From 115293e856f23d03f84a836e0fb82547b1c13b07 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Mon, 11 Feb 2013 16:29:33 +0100
Subject: [PATCH 6/8] Remove legacy toDict and origDataDict methods of
 LDAPEntry.

---
 ipaserver/install/krbinstance.py |  4 ++--
 ipaserver/install/ldapupdate.py  |  7 +++----
 ipaserver/ipaldap.py             | 31 ++++---------------------------
 3 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 51c5427..4bf291f 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -124,10 +124,10 @@ class KrbInstance(service.Service):
             ipauniqueid=['autogenerate'],
             managedby=[host_dn],
         )
-        if 'krbpasswordexpiration' in service_entry.toDict():
+        if 'krbpasswordexpiration' in service_entry:
             host_entry['krbpasswordexpiration'] = service_entry[
                 'krbpasswordexpiration']
-        if 'krbticketflags' in service_entry.toDict():
+        if 'krbticketflags' in service_entry:
             host_entry['krbticketflags'] = service_entry['krbticketflags']
         self.admin_conn.add_entry(host_entry)
 
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 2f2e7de..643d390 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -783,11 +783,10 @@ class LDAPUpdate:
         else:
             # Update LDAP
             try:
-                changes = self.conn.generateModList(entry.origDataDict(), entry.toDict())
+                changes = self.conn.generateModList(entry.orig_data, entry)
                 if (entry.dn == DN(('cn', 'schema'))):
                     d = dict()
-                    e = entry.toDict()
-                    for k,v in e.items():
+                    for k,v in entry.items():
                         d[k] = [str(x) for x in v]
                     updated = self.is_schema_updated(d)
                 else:
@@ -796,7 +795,7 @@ class LDAPUpdate:
                 self.debug("%s" % changes)
                 self.debug("Live %d, updated %d" % (self.live_run, updated))
                 if self.live_run and updated:
-                    self.conn.updateEntry(entry.dn, entry.origDataDict(), entry.toDict())
+                    self.conn.updateEntry(entry.dn, entry.orig_data, entry)
                 self.info("Done")
             except errors.EmptyModlist:
                 self.info("Entry already up-to-date")
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 70f0ec0..ca279b7 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1021,26 +1021,6 @@ class LDAPEntry(object):
         yield self._dn
         yield self
 
-    def toDict(self):
-        # FIXME: for backwards compatibility only
-        """Convert the attrs and values to a dict. The dict is keyed on the
-        attribute name.  The value is either single value or a list of values."""
-        assert isinstance(self.dn, DN)
-        result = ipautil.CIDict(self.data)
-        for i in result.keys():
-            result[i] = ipautil.utf8_encode_values(result[i])
-        result['dn'] = self.dn
-        return result
-
-    def origDataDict(self):
-        """Returns a dict of the original values of the user.
-
-        Used for updates.
-        """
-        result = ipautil.CIDict(self.orig_data)
-        result['dn'] = self.dn
-        return result
-
 
 class LDAPClient(object):
     """LDAP backend class
@@ -1934,11 +1914,8 @@ class IPAdmin(LDAPClient):
         FORCE_REPLACE_ON_UPDATE_ATTRS = ('nsslapd-ssl-check-hostname', 'nsslapd-lookthroughlimit', 'nsslapd-idlistscanlimit', 'nsslapd-anonlimitsdn', 'nsslapd-minssf-exclude-rootdse')
         modlist = []
 
-        old_entry = ipautil.CIDict(old_entry)
-        new_entry = ipautil.CIDict(new_entry)
-
-        keys = set(map(string.lower, old_entry.keys()))
-        keys.update(map(string.lower, new_entry.keys()))
+        keys = set(old_entry.keys())
+        keys.update(new_entry.keys())
 
         for key in keys:
             new_values = new_entry.get(key, [])
@@ -1967,9 +1944,9 @@ class IPAdmin(LDAPClient):
 
             # You can't remove schema online. An add will automatically
             # replace any existing schema.
-            if old_entry.get('dn', DN()) == DN(('cn', 'schema')):
+            if old_entry.dn == DN(('cn', 'schema')):
                 if len(adds) > 0:
-                    if key == 'attributetypes':
+                    if key.lower() == 'attributetypes':
                         modlist.insert(0, (ldap.MOD_ADD, key, adds))
                     else:
                         modlist.append((ldap.MOD_ADD, key, adds))
-- 
1.8.1

>From 1d574ca61b8fb391344c0acf4b9d729deb216565 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Wed, 27 Feb 2013 16:01:44 +0100
Subject: [PATCH 7/8] Save raw attribute values of search results in
 IPASimpleLDAPObject.

---
 ipaserver/ipaldap.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index ca279b7..f39ea6d 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -410,7 +410,7 @@ class IPASimpleLDAPObject(object):
             ipa_entry = LDAPEntry(self, DN(original_dn))
 
             for attr, original_values in original_attrs.items():
-                ipa_entry[attr] = self.decode(original_values, attr)
+                ipa_entry.set_raw(attr, original_values)
 
             ipa_result.append(ipa_entry)
 
-- 
1.8.1

>From 4297435d5ec67725c595f6c140d29db8aea2a865 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <[email protected]>
Date: Wed, 27 Feb 2013 16:07:15 +0100
Subject: [PATCH 8/8] Use raw attribute values when generating modlists in
 LDAPClient and IPAdmin.

---
 ipaserver/ipaldap.py | 35 ++++-------------------------------
 1 file changed, 4 insertions(+), 31 deletions(-)

diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index f39ea6d..5cf7a02 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1707,32 +1707,12 @@ class LDAPClient(object):
         # updates better
         # for single value attribute: always MOD_REPLACE
         modlist = []
-        for (k, v) in entry_attrs.iteritems():
+        for (k, v) in entry_attrs.iteritems_raw():
             if v is None and k in entry_attrs_old:
                 modlist.append((ldap.MOD_DELETE, k, None))
             else:
-                if not isinstance(v, (list, tuple)):
-                    v = [v]
                 v = set(filter(lambda value: value is not None, v))
-                old_v = set(entry_attrs_old.get(k.lower(), []))
-
-                # FIXME: Convert all values to either unicode, DN or str
-                # before detecting value changes (see IPASimpleLDAPObject for
-                # supported types).
-                # This conversion will set a common ground for the comparison.
-                #
-                # This fix can be removed when ticket 2265 is fixed and our
-                # encoded entry_attrs' types will match get_entry result
-                try:
-                    v = set(
-                        unicode_from_utf8(self.conn.encode(value))
-                        if not isinstance(value, (DN, str, unicode))
-                        else value for value in v)
-                except Exception, e:
-                    # Rather let the value slip in modlist than let ldap2 crash
-                    self.log.error(
-                        "Cannot convert attribute '%s' for modlist "
-                        "for modlist comparison: %s", k, e)
+                old_v = set(entry_attrs_old.get_raw(k.lower()))
 
                 adds = list(v.difference(old_v))
                 rems = list(old_v.difference(v))
@@ -1918,15 +1898,8 @@ class IPAdmin(LDAPClient):
         keys.update(new_entry.keys())
 
         for key in keys:
-            new_values = new_entry.get(key, [])
-            if not(isinstance(new_values,list) or isinstance(new_values,tuple)):
-                new_values = [new_values]
-            new_values = filter(lambda value:value!=None, new_values)
-
-            old_values = old_entry.get(key, [])
-            if not(isinstance(old_values,list) or isinstance(old_values,tuple)):
-                old_values = [old_values]
-            old_values = filter(lambda value:value!=None, old_values)
+            new_values = new_entry.get_raw(key)
+            old_values = old_entry.get_raw(key)
 
             # We used to convert to sets and use difference to calculate
             # the changes but this did not preserve order which is important
-- 
1.8.1

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to