On 08/12/2010 09:35 AM, Pavel Zůna wrote:
On 2010-08-12 14:38, Rob Crittenden wrote:
Pavel Zůna wrote:
On 2010-08-12 04:46, Rob Crittenden wrote:
Pavel Zůna wrote:
setattr and addattr can now be used both to set all values of
ANY attribute. the last setattr always resets the attribute to
the specified value and all addattr append to it.

Examples:
user-mod testuser --setattr=title=msc
title: msc
user-mod testuser --setattr=title=msb
title: msb
user-mod testuser --addattr=title=msc
title: msb, msc
user-mod testuser --setattr=title=
title:
user-mod testuser --setattr=title=msc --addattr=msb
title: msc, msb
user-mod testuser --setattr=title=ing --addattr=bc
title: ing, bc
user-mod testuser --setattr=title=doc
title: doc

It's not very user friendly, but it's going to be used very very
rarely in special conditions in the CLI and we can use it to save
lots of JSON-RPC roundtrips in the webUI.

Pavel

It was my intention when I added addattr and setattr that one couldn't
set already-defined params this way. They were silently ignored. So you
couldn't do:

user-mod testuser --setattr=givenname=Jeff

This would be possible with this patch. Was that intentional?

BTW I have the start of a test suite for this functionality.

rob

Yes, it is intentional. I forgot to mention it in the description. I'm
using setattr/addattr for everything in the webUI - it makes the code a
lot simpler.

Doesn't that invalidate all the validators we have in the plugins? This
is why I disallowed it.

rob

It does, but I see these options as something only experienced users, who need to set something we don't support directly, will use. Sometimes they might want to disable the validators, if they know what they're doing. We could also make the setattr/addattr handler in frontend.py detect if a there's a validator available and use it.

Validators in the webUI is still something we need to figure out. Adam was proposing having validators in the form of regex strings, which is not a bad idea as it's easy to implement on any platform/language. On the other hand, I don't know if it's good enough for all parameters we have.

Hmm. There's a lot to think about here actually. I'll make it my homework for the weekend. :)

Pavel

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


Here's my version, that just calls the parameter prior to updating the attr. I tested it with:

[r...@ipa ~]# ipa user-mod --setattr uidnumber=555 kfrog
---------------------
Modified user "kfrog"
---------------------
  User login: kfrog
  First name: Kermit
  Last name: Frog
  Home directory: /home/kfrog
  Login shell: /bin/sh
  UID: 555
  Groups: ipausers
[r...@ipa ~]# ipa user-mod --setattr uidnumber=frog kfrog
ipa: ERROR: invalid 'uidnumber': must be an integer



>From 030b5dab93971495d8656f7886c29136e118a9e6 Mon Sep 17 00:00:00 2001
From: Adam Young <ayo...@redhat.com>
Date: Fri, 13 Aug 2010 16:20:41 -0400
Subject: [PATCH] Change the behaviour of addattr/setattr parameters.

setattr and addattr can now be used both to set all values of
ANY attribute. the last setattr always resets the attribute to
the specified value and all addattr append to it.

Examples:
user-mod testuser --setattr=title=msc
  title: msc
user-mod testuser --setattr=title=msb
  title: msb
user-mod testuser --addattr=title=msc
  title: msb, msc
user-mod testuser --setattr=title=
  title:
user-mod testuser --setattr=title=msc --addattr=msb
  title: msc, msb
user-mod testuser --setattr=title=ing --addattr=bc
  title: ing, bc
user-mod testuser --setattr=title=doc
  title: doc

It's not very user friendly, but it's going to be used very very
rarely in special conditions in the CLI and we can use it to save
lots of JSON-RPC roundtrips in the webUI.

This version includes calling the validation of Params during the setting of the attrs.
---
 ipalib/frontend.py         |   17 ++++++++----
 ipalib/plugins/baseldap.py |   58 ++++++++++++++++++++++----------------------
 2 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index d320f02..1c4fea8 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -519,11 +519,12 @@ class Command(HasParam):
             if len(value) == 0:
                 # None means "delete this attribute"
                 value = None
-            if attr not in self.params:
-                if append and attr in newdict:
-                    newdict[attr].append(value)
-                else:
-                    newdict[attr] = [value]
+            if attr in self.params:
+                value = self.params[attr](value)
+            if append and attr in newdict:
+                newdict[attr].append(value)
+            else:
+                newdict[attr] = [value]
         return newdict
 
     def __attributes_2_entry(self, kw):
@@ -540,7 +541,11 @@ class Command(HasParam):
             adddict = self.__convert_2_dict(kw['setattr'], append=False)
 
         if kw.get('addattr'):
-            adddict.update(self.__convert_2_dict(kw['addattr']))
+            for (k, v) in self.__convert_2_dict(kw['addattr']).iteritems():
+                if k in adddict:
+                    adddict[k] += v
+                else:
+                    adddict[k] = v
 
         for name in adddict:
             value = adddict[name]
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 43fafe3..f3e5b0f 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -419,6 +419,35 @@ class LDAPUpdate(LDAPQuery, crud.Update):
 
         entry_attrs = self.args_options_2_entry(**options)
 
+        """
+        Some special handling is needed because we need to update the
+        values here rather than letting ldap.update_entry() do the work. We
+        have to do the work of adding new values to an existing attribute
+        because if we pass just what is addded only the new values get
+        set.
+        """
+        if 'addattr' in options:
+            setset = set(get_attributes(options.get('setattr', [])))
+            addset = set(get_attributes(options.get('addattr', [])))
+            difflist = list(addset.difference(setset))
+            if difflist:
+                try:
+                    (dn, old_entry) = ldap.get_entry(
+                        dn, difflist, normalize=self.obj.normalize_dn
+                    )
+                except errors.ExecutionError, e:
+                    try:
+                        (dn, old_entry) = self._call_exc_callbacks(
+                            keys, options, e, ldap.get_entry, dn, attrs_list,
+                            normalize=self.obj.normalize_dn
+                        )
+                    except errors.NotFound:
+                        self.obj.handle_not_found(*keys)
+                for a in old_entry:
+                    if not isinstance(entry_attrs[a], (list, tuple)):
+                        entry_attrs[a] = [entry_attrs[a]]
+                    entry_attrs[a] += old_entry[a]
+
         if options.get('all', False):
             attrs_list = ['*']
         else:
@@ -436,35 +465,6 @@ class LDAPUpdate(LDAPQuery, crud.Update):
                     self, ldap, dn, entry_attrs, attrs_list, *keys, **options
                 )
 
-        """
-        Some special handling is needed because we need to update the
-        values here rather than letting ldap.update_entry() do the work. We
-        have to do the work of adding new values to an existing attribute
-        because if we pass just what is addded only the new values get
-        set.
-        """
-        if 'addattr' in options:
-            try:
-                (dn, old_entry) = ldap.get_entry(
-                    dn, attrs_list, normalize=self.obj.normalize_dn
-                )
-            except errors.ExecutionError, e:
-                try:
-                    (dn, old_entry) = self._call_exc_callbacks(
-                        keys, options, e, ldap.get_entry, dn, attrs_list,
-                        normalize=self.obj.normalize_dn
-                    )
-                except errors.NotFound:
-                    self.obj.handle_not_found(*keys)
-            attrlist = get_attributes(options['addattr'])
-            for attr in attrlist:
-                if attr in old_entry:
-                    if type(entry_attrs[attr]) in (tuple,list):
-                        entry_attrs[attr] = old_entry[attr] + entry_attrs[attr]
-                    else:
-                        old_entry[attr].append(entry_attrs[attr])
-                        entry_attrs[attr] = old_entry[attr]
-
         try:
             ldap.update_entry(dn, entry_attrs, normalize=self.obj.normalize_dn)
         except errors.ExecutionError, e:
-- 
1.7.1

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

Reply via email to