On 04/09/2015 12:42 PM, thierry bordaz wrote:
On 04/08/2015 03:33 PM, Jan Cholasta wrote:
Dne 8.4.2015 v 15:00 thierry bordaz napsal(a):
On 04/08/2015 08:34 AM, Jan Cholasta wrote:

Dne 1.4.2015 v 17:40 thierry bordaz napsal(a):

In user life cycle, Active entries are moved to Delete container and
    Delete entries can be moved back to Staging container.
This requires a LDAP modrdn with new superior that is not supported
    in ldap2.

Since update_entry_rdn() is used only in one spot in baseldap, I think
we can merge it and move_entry_newsuperior() into a single method

    def move_entry(self, dn, new_dn, del_old=True):

We can easily detect whether the superior needs to be updated by
comparing dn[1:] and new_dn[1:].

Hello Jan,

Yes that is a good idea to merge those two methods. They both rely on
modrdn and a single method is enough.

Well, I had something like this in mind:

    def move_entry(self, dn, new_dn, del_old=True):
        assert isinstance(dn, DN)
        assert isinstance(new_dn, DN)

        if new_dn == dn:
            raise errors.EmptyModlist()

        new_rdn = new_dn[0]
        if new_rdn == dn[0]:
            new_rdn = None

        new_superior = new_dn[1:]
        if new_superior == dn[1:]:
            new_superior = None

        with self.error_handler():
            self.conn.rename_s(dn, new_rdn, new_superior, int(del_old))
            time.sleep(.3)  # Give memberOf plugin a chance to work

so that you don't have to care if you should change the RDN or the superior and it just does the right thing.

Maybe we can also get rid of del_old, if it's always gonna be True in
our code?

I think it is better to get this interface as close as possible as the
MODRDN call, so that del_old option will be already available for future
I agree that currently del_old is always true in case of IPA but it
could be the default value.

OK, it's not a big piece of code, so I guess we can leave it.

Thank for the reviews and the help. Here is a new patch.



After additional tests, the previous patch was incomplete...

From b6382bc4a78642a0b08fdbe193deba780ff6c63a Mon Sep 17 00:00:00 2001
From: "Thierry bordaz (tbordaz)" <tbor...@redhat.com>
Date: Wed, 1 Apr 2015 16:42:43 +0200
Subject: [PATCH 07/13] User life cycle: allows MODRDN from ldap2

enhance update_entry_rdn so that is allows
to move an entry a new superior

Reviewed By: Jan Cholasta

 ipalib/plugins/baseldap.py |  6 +++---
 ipapython/ipaldap.py       | 28 ++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 4b1c701924d57919538e0c428ea181c2e898505e..127c875b16e3cadd2a73c154e56d0385e1b342e8 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1417,10 +1417,10 @@ class LDAPUpdate(LDAPQuery, crud.Update):
             if self.obj.rdn_is_primary_key and self.obj.primary_key.name in entry_attrs:
                     # RDN change
-                    self._exc_wrapper(keys, options, ldap.update_entry_rdn)(
+                    new_dn = DN((self.obj.primary_key.name, entry_attrs[self.obj.primary_key.name]), *entry_attrs.dn[1:])
+                    self._exc_wrapper(keys, options, ldap.move_entry)(
-                        RDN((self.obj.primary_key.name,
-                             entry_attrs[self.obj.primary_key.name])))
+                        new_dn)
                     rdnkeys = keys[:-1] + (entry_attrs[self.obj.primary_key.name], )
                     entry_attrs.dn = self.obj.get_dn(*rdnkeys)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index ce07006eb790c80fd42bd6eb611732ce9000db13..acab52c0b4011edf0dadab11f656469c2fc17130 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -581,6 +581,9 @@ class IPASimpleLDAPObject(object):
         dn = str(dn)
         assert isinstance(newrdn, (DN, RDN))
         newrdn = str(newrdn)
+        if newsuperior:
+            assert isinstance(newsuperior, DN)
+            newsuperior = str(newsuperior)
         return self.conn.rename_s(dn, newrdn, newsuperior, delold)
     def result(self, msgid=ldap.RES_ANY, all=1, timeout=None):
@@ -1593,21 +1596,34 @@ class LDAPClient(object):
-    def update_entry_rdn(self, dn, new_rdn, del_old=True):
+    def move_entry(self, dn, new_dn, del_old=True):
-        Update entry's relative distinguished name.
+        Move an entry (either to a new superior or/and changing relative distinguished name)
         Keyword arguments:
+        dn: DN of the source entry
+        new_dn: DN of the target entry
         del_old -- delete old RDN value (default True)
+        :raises:
+        errors.NotFound if source entry or target superior entry doesn't exist
+        errors.EmptyModlist if source and target are identical
         assert isinstance(dn, DN)
-        assert isinstance(new_rdn, RDN)
+        assert isinstance(new_dn, DN)
-        if dn[0] == new_rdn:
+        if new_dn == dn:
             raise errors.EmptyModlist()
+        new_rdn = new_dn[0]
+        if new_dn[1:] == dn[1:]:
+            new_superior = None
+        else:
+            new_superior = DN(*new_dn[1:])
         with self.error_handler():
-            self.conn.rename_s(dn, new_rdn, delold=int(del_old))
+            self.conn.rename_s(dn, new_rdn, newsuperior=new_superior, delold=int(del_old))
             time.sleep(.3)  # Give memberOf plugin a chance to work
     def update_entry(self, entry, entry_attrs=None):

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to