On 02/19/2013 02:17 PM, Jan Cholasta wrote:
On 29.1.2013 10:21, Jan Cholasta wrote:
A patch from this patchset (part 3) causes some of the dns plugin tests
to fail (idnsallowdynupdate is missing in dnszone_add output).
Honza
Patch 143:
+ assert isinstance(entry_or_dn, DN)
+ if normalize is None or normalize:
+ entry_or_dn = self.normalize_dn(entry_or_dn)
+ entry_attrs = dict(entry_attrs)
Can you please return LDAPEntry here as well, i.e. replace
dict(entry_attrs) with self.make_entry(entry_or_dn, entry_attrs)?
Sure. I tried to keep the old behavior with old usage, but you're right,
using LDAPEntry here will be better.
+ def delete_entry(self, entry, normalize=None):
+ """Delete entry.
+
+ The `normalize` argument does nothing when called with a
LDAPEntry.
+
+ The legacy variant is:
+ delete_entry(dn, normalize=True)
+ """
I don't think this is right. We don't need to know any of the attributes
of an entry to delete it, just its DN. I think we should keep the DN
variant of delete_entry as the primary one.
Makes sense. I made them both primary.
I didn't bother documenting normalize since your patch will remove that.
Updated patch attached; I'll update my repo when I respond to your
comments on part 4.
--
Petr³
From e0ea08a85d211198a7d745e944bea6d999f2317d Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 23 Jan 2013 06:38:32 -0500
Subject: [PATCH] Change {add,update,delete}_entry to take LDAPEntries
These methods currently take (dn, entry_attrs, normalize=True)
(or (dn, normalize=True) for delete).
Change them to also accept just an LDAPEntry.
For add and update, document the old style as deprecated.
Part of the work for: https://fedorahosted.org/freeipa/ticket/2660
---
ipaserver/ipaldap.py | 79 +++++++++++++++++++++++++++++++------------------
1 files changed, 50 insertions(+), 29 deletions(-)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index b496bfbb480f2cc2c7b22397d475d725f7f75eea..95e466f6e8ff6007f7e18ac071cfde5ff3bfa718 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1364,21 +1364,40 @@ class LDAPConnection(object):
self.log.debug("get_members: result=%s", entries)
return entries
- def add_entry(self, dn, entry_attrs, normalize=True):
- """Create a new entry."""
-
- assert isinstance(dn, DN)
-
- if normalize:
- dn = self.normalize_dn(dn)
- # remove all None or [] values, python-ldap hates'em
- entry_attrs = dict(
- # FIXME, shouldn't these values be an error?
- (k, v) for (k, v) in entry_attrs.iteritems()
- if v is not None and v != []
- )
+ def _get_dn_and_attrs(self, entry_or_dn, entry_attrs, normalize):
+ """Helper for legacy calling style for {add,update}_entry
+ """
+ if entry_attrs is None:
+ assert normalize is None
+ return entry_or_dn.dn, entry_or_dn
+ else:
+ assert isinstance(entry_or_dn, DN)
+ if normalize is None or normalize:
+ entry_or_dn = self.normalize_dn(entry_or_dn)
+ entry_attrs = self.make_entry(entry_or_dn, entry_attrs)
+ for key, value in entry_attrs.items():
+ if value is None:
+ entry_attrs[key] = []
+ return entry_or_dn, entry_attrs
+
+ def add_entry(self, entry, entry_attrs=None, normalize=None):
+ """Create a new entry.
+
+ This should be called as add_entry(entry).
+
+ The legacy two/three-argument variant is:
+ add_entry(dn, entry_attrs, normalize=True)
+ """
+ dn, attrs = self._get_dn_and_attrs(entry, entry_attrs, normalize)
+
+ # remove all [] values (python-ldap hates 'em)
+ attrs = dict((k, v) for k, v in attrs.iteritems()
+ # FIXME: Once entry values are always lists, this condition can
+ # be just "if v":
+ if v is not None and v != [])
+
try:
- self.conn.add_s(dn, list(entry_attrs.iteritems()))
+ self.conn.add_s(dn, list(attrs.iteritems()))
except _ldap.LDAPError, e:
self.handle_errors(e)
@@ -1465,34 +1484,36 @@ class LDAPConnection(object):
return modlist
- def update_entry(self, dn, entry_attrs, normalize=True):
- """
- Update entry's attributes.
+ def update_entry(self, entry, entry_attrs=None, normalize=None):
+ """Update entry's attributes.
- An attribute value set to None deletes all current values.
- """
+ This should be called as update_entry(entry).
- assert isinstance(dn, DN)
- if normalize:
- dn = self.normalize_dn(dn)
+ The legacy two/three-argument variant is:
+ update_entry(dn, entry_attrs, normalize=True)
+ """
+ dn, attrs = self._get_dn_and_attrs(entry, entry_attrs, normalize)
# generate modlist
- modlist = self._generate_modlist(dn, entry_attrs, normalize)
+ modlist = self._generate_modlist(dn, attrs, normalize)
if not modlist:
raise errors.EmptyModlist()
# pass arguments to python-ldap
try:
self.conn.modify_s(dn, modlist)
except _ldap.LDAPError, e:
self.handle_errors(e)
- def delete_entry(self, dn, normalize=True):
- """Delete entry."""
-
- assert isinstance(dn, DN)
- if normalize:
- dn = self.normalize_dn(dn)
+ def delete_entry(self, entry_or_dn, normalize=None):
+ """Delete an entry given either the DN or the entry itself"""
+ if isinstance(entry_or_dn, DN):
+ dn = entry_or_dn
+ if normalize is None or normalize:
+ dn = self.normalize_dn(dn)
+ else:
+ assert normalize is None
+ dn = entry_or_dn.dn
try:
self.conn.delete_s(dn)
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
