On 01/16/2013 04:45 PM, Jan Cholasta wrote:
Hi,

this patch adds initial support for custom LDAP entry objects, as
described in <http://freeipa.org/page/V3/LDAP_code>.

The LDAPEntry class implements the mapping object. The current version
works like a dict, plus it has a DN property and validates and
normalizes attribute names (there is no case preservation yet).

The LDAPEntryCompat class provides compatibility with old code that uses
(dn, attrs) tuples. The reason why this is a separate class is that our
code currently has 2 contradicting requirements on the entry object:
tuple unpacking must work with it (i.e. iter(entry) yields dn and
attribute dict), but it also must work as an argument to dict
constructor (i.e. iter(entry) yields attribute names). This class will
be removed once our code is converted to use LDAPEntry.

With this patch, ldap2 produces LDAPEntryCompat objects. I don't see how that brings us closer to converting the codebase to LDAPEntry. I'd be happier if this patch made both the tuple unpacking and dict-style access possible. But perhaps you have a better plan than I can see from the patch?


The dict constructor uses the `keys` method, not iteration, so these two requirements aren't incompatible -- unless we do specifically require that iter(entry) yields attribute names. For example the following will work:

    class NotQuiteADict(dict):
        def __iter__(self):
            return iter(('some', 'data'))

    a = NotQuiteADict(a=1, b=2, c=3)
    print dict(a)  # -> {'a': 1, 'c': 3, 'b': 2}
    print list(a)  # -> ['some', 'data']
    print a.keys()  # -> ['a', 'c', 'b']


While overriding a dict's __iter__ in this way is a bit evil, I think for our purposes it would be better than introducing a fourth entry class. When all our code is converted we could just have __iter__ warn or raise an exception for a few releases to make sure no one uses it.

In a couple of places we do iterate over the entry to get the keys, but not in many, and they're usually well-tested. We just have to use `entry.keys()` there.

Would these changes to your patch be OK?



Another issue is that the DN is itself an attribute, so entry.dn and entry['dn'] should be synchronized. I guess that's material for another patch, though.

--
PetrĀ³

From 3f95d92ee11dcee61456b7a1f81e8ab0f282dc8d Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 16 Jan 2013 14:14:58 +0100
Subject: [PATCH] Add custom mapping object for LDAP entry data.

---
 ipalib/plugins/automember.py      |    2 +-
 ipalib/plugins/baseldap.py        |    8 +--
 ipalib/plugins/dns.py             |   11 ++--
 ipalib/plugins/permission.py      |    2 +-
 ipaserver/ipaldap.py              |    4 +-
 ipaserver/plugins/ldap2.py        |  105 ++++++++++++++++++++++++++++++++-----
 tests/test_ipaserver/test_ldap.py |   34 ++++++++++++-
 7 files changed, 138 insertions(+), 28 deletions(-)

diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index 0a7f37319b380f054b3a3a77cc06b72813b9d910..df2a1dce0253ab364b6b57b4559a5aea388e830c 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -305,7 +305,7 @@ class automember_add_condition(LDAPUpdate):
                 try:
                     (dn, old_entry) = ldap.get_entry(
                         dn, [attr], normalize=self.obj.normalize_dn)
-                    for regex in old_entry:
+                    for regex in old_entry.keys():
                         if not isinstance(entry_attrs[regex], (list, tuple)):
                             entry_attrs[regex] = [entry_attrs[regex]]
                         duplicate = set(old_entry[regex]) & set(entry_attrs[regex])
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 85e2bec36b011fd4539dfadc8d97535bb157bca7..30e2cab2cf6da1f3f8c128f8f81552a43ada3df0 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -224,12 +224,10 @@ def entry_from_entry(entry, newentry):
     """
 
     # Wipe out the current data
-    for e in entry.keys():
-        del entry[e]
+    entry.clear()
 
     # Re-populate it with new wentry
-    for e in newentry:
-        entry[e] = newentry[e]
+    entry.update(newentry)
 
 def wait_for_value(ldap, dn, attr, value):
     """
@@ -868,7 +866,7 @@ last, after all sets and adds."""),
             # Provide a nice error message when user tries to delete an
             # attribute that does not exist on the entry (and user is not
             # adding it)
-            names = set(n.lower() for n in old_entry)
+            names = set(n.lower() for n in old_entry.keys())
             del_nonexisting = delattrs - (names | setattrs | addattrs)
             if del_nonexisting:
                 raise errors.ValidationError(name=del_nonexisting.pop(),
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index ae72f30532dcc1c88358ef2a45e0e935c721358b..af5d56fa05ad0a7060c3119e8042523b7653e504 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2267,9 +2267,10 @@ class dnsrecord(LDAPObject):
 
     def check_record_type_collisions(self, old_entry, entry_attrs):
         # Test that only allowed combination of record types was created
-        attrs = set(attr for attr in entry_attrs if attr in _record_attributes
-                        and entry_attrs[attr])
-        attrs.update(attr for attr in old_entry if attr not in entry_attrs)
+        attrs = set(attr for attr in entry_attrs.keys()
+            if attr in _record_attributes and entry_attrs[attr])
+        attrs.update(attr for attr in old_entry.keys()
+            if attr not in entry_attrs)
         try:
             attrs.remove('cnamerecord')
         except KeyError:
@@ -2568,7 +2569,7 @@ class dnsrecord_mod(LDAPUpdate):
                     normalize=self.obj.normalize_dn)
 
             del_all = True
-            for attr in old_entry:
+            for attr in old_entry.keys():
                 if old_entry[attr]:
                     del_all = False
                     break
@@ -2709,7 +2710,7 @@ class dnsrecord_del(LDAPUpdate):
         del_all = False
         if not self.obj.is_pkey_zone_record(*keys):
             record_found = False
-            for attr in old_entry:
+            for attr in old_entry.keys():
                 if old_entry[attr]:
                     record_found = True
                     break
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 1fbf9e018b06c914614c946c9c345c7fe256398e..37094c8d37006e4b5b424421b9caa0b111985890 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -400,7 +400,7 @@ class permission_mod(LDAPUpdate):
         common_options = filter_options(options, ['all', 'raw', 'rights'])
         result = self.api.Command.permission_show(cn, **common_options)['result']
 
-        for r in result:
+        for r in result.keys():
             if not r.startswith('member_'):
                 entry_attrs[r] = result[r]
         return dn
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 9b3b86fcee9c35515b9d782d9608f9f0cfd4ae88..852cbc306ca362944729b9635c15695b5fa00ee7 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -40,7 +40,7 @@ from ipalib import errors
 from ipapython.ipautil import format_netloc, wait_for_open_socket, wait_for_open_ports
 from ipapython.dn import DN
 from ipapython.entity import Entity
-from ipaserver.plugins.ldap2 import IPASimpleLDAPObject
+from ipaserver.plugins.ldap2 import IPASimpleLDAPObject, LDAPEntry
 
 # Global variable to define SASL auth
 SASL_AUTH = ldap.sasl.sasl({},'GSSAPI')
@@ -102,7 +102,7 @@ class Entry:
         a search result entry or a reference or None.
         If creating a new empty entry, data is the string DN."""
         if entrydata:
-            if isinstance(entrydata,tuple):
+            if isinstance(entrydata, (tuple, LDAPEntry)):
                 self.dn = entrydata[0]
                 self.data = ipautil.CIDict(entrydata[1])
             elif isinstance(entrydata,DN):
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 8e8e1604ff0a3d36fe3501ec6f54abdb717d78ae..4b1fe6e2302941aa315a5c1d44103e33abecc8db 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -47,7 +47,6 @@ import ldap.filter as _ldap_filter
 import ldap.sasl as _ldap_sasl
 from ipapython.dn import DN, RDN
 from ipapython.ipautil import CIDict
-from collections import namedtuple
 from ipalib.errors import NetworkError, DatabaseError
 
 
@@ -75,14 +74,95 @@ from ipalib.request import context
 
 _debug_log_ldap = False
 
-# Make python-ldap tuple style result compatible with Entry and Entity
-# objects by allowing access to the dn (tuple index 0) via the 'dn'
-# attribute name and the attr dict (tuple index 1) via the 'data'
-# attribute name. Thus:
-# r = result[0]
-# r[0] == r.dn
-# r[1] == r.data
-LDAPEntry = namedtuple('LDAPEntry', ['dn', 'data'])
+class LDAPEntry(dict):
+    __slots__ = ('_dn',)
+
+    def __init__(self, _dn=None, _obj=None, **kwargs):
+        if isinstance(_dn, LDAPEntry):
+            assert _obj is None
+            _obj = _dn
+            self._dn = DN(_obj._dn)    #pylint: disable=E1103
+        else:
+            assert isinstance(_dn, DN)
+            if _obj is None:
+                _obj = {}
+            self._dn = _dn
+
+        super(LDAPEntry, self).__init__(self._init_iter(_obj, **kwargs))
+
+    @property
+    def dn(self):
+        return self._dn
+
+    @dn.setter
+    def dn(self, value):
+        assert isinstance(value, DN)
+        self._dn = value
+
+    def _attr_name(self, name):
+        if isinstance(name, DN):
+            name = str(name)
+        if not isinstance(name, basestring):
+            raise TypeError(
+                "attribute name must be unicode or str, got %s object %r" % (
+                    name.__class__.__name__, name))
+        if isinstance(name, str):
+            name = name.decode('ascii')
+        return name.lower()
+
+    def _init_iter(self, _obj, **kwargs):
+        _obj = dict(_obj, **kwargs)
+        for (k, v) in _obj.iteritems():
+            yield (self._attr_name(k), v)
+
+    def __repr__(self):
+        dict_repr = super(LDAPEntry, self).__repr__()
+        return '%s(%s, %s)' % (type(self).__name__, repr(self._dn), dict_repr)
+
+    def copy(self):
+        return LDAPEntry(self)
+
+    def __setitem__(self, name, value):
+        super(LDAPEntry, self).__setitem__(self._attr_name(name), value)
+
+    def setdefault(self, name, default):
+        return super(LDAPEntry, self).setdefault(self._attr_name(name), default)
+
+    def update(self, _obj={}, **kwargs):
+        super(LDAPEntry, self).update(self._init_iter(_obj, **kwargs))
+
+    def __getitem__(self, name):
+        # FIXME: For backwards compatibility, entry[0] and entry[1] return
+        # the dn and the entry itself, respectively
+        if name == 0:
+            return self.dn
+        if name == 1:
+            return self
+        return super(LDAPEntry, self).__getitem__(self._attr_name(name))
+
+    def get(self, name, default=None):
+        return super(LDAPEntry, self).get(self._attr_name(name), default)
+
+    def __delitem__(self, name):
+        super(LDAPEntry, self).__delitem__(self._attr_name(name))
+
+    def pop(self, name, *default):
+        return super(LDAPEntry, self).pop(self._attr_name(name), *default)
+
+    def __contains__(self, name):
+        return super(LDAPEntry, self).__contains__(self._attr_name(name))
+
+    def has_key(self, name):
+        return super(LDAPEntry, self).has_key(self._attr_name(name))
+
+    @property
+    def data(self):
+        # FIXME: for backwards compatibility only
+        return self
+
+    def __iter__(self):
+        # FIXME: for backwards compatibility only
+        return iter((self.dn, self))
 
 
 # Group Member types
@@ -454,14 +534,13 @@ class IPASimpleLDAPObject(object):
             original_dn = dn_tuple[0]
             original_attrs = dn_tuple[1]
 
-            ipa_dn = DN(original_dn)
-            ipa_attrs = dict()
+            ipa_entry = LDAPEntry(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_attrs[attr.lower()] = self.convert_value_list(attr, target_type, original_values)
+                ipa_entry[attr] = self.convert_value_list(attr, target_type, original_values)
 
-            ipa_result.append(LDAPEntry(ipa_dn, ipa_attrs))
+            ipa_result.append(ipa_entry)
 
         if _debug_log_ldap:
             self.debug('ldap.result: %s', ipa_result)
diff --git a/tests/test_ipaserver/test_ldap.py b/tests/test_ipaserver/test_ldap.py
index cd3ba3cd5a9485081e25fe077af73269ed84968d..c9eee78af006f185dd19f3d453d7482210464d3d 100644
--- a/tests/test_ipaserver/test_ldap.py
+++ b/tests/test_ipaserver/test_ldap.py
@@ -27,7 +27,7 @@
 
 import nose
 import os
-from ipaserver.plugins.ldap2 import ldap2
+from ipaserver.plugins.ldap2 import ldap2, LDAPEntry
 from ipalib.plugins.service import service, service_show
 from ipalib.plugins.host import host
 import nss.nss as nss
@@ -145,3 +145,35 @@ class test_ldap(object):
         cert = cert[0]
         serial = unicode(x509.get_serial_number(cert, x509.DER))
         assert serial is not None
+
+    def test_entry(self):
+        """
+        Test the LDAPEntry class
+        """
+        cn1 = [u'test1']
+        cn2 = [u'test2']
+        dn1 = DN(('cn', cn1[0]))
+        dn2 = DN(('cn', cn2[0]))
+
+        e = LDAPEntry(dn1, cn=cn1)
+        assert e.dn is dn1
+        assert 'CN' in e
+        assert e['CN'] is cn1
+        assert e['CN'] is e[u'cn']
+
+        e.dn = dn2
+        assert e.dn is dn2
+
+        e['cn'] = cn2
+        assert 'CN' in e
+        assert e['CN'] is cn2
+        assert e['CN'] is e[u'cn']
+
+        del e['CN']
+        assert 'CN' not in e
+        assert u'cn' not in e
+
+        # Backwards compatibility
+        assert e[0] is e.dn
+        assert e[1] is e.data is e
+        assert tuple(e) == (e.dn, e)
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to