When using *attr we should return the param.name of in the exception and when using a cli option we should return param.cli_name. This didn't work consistently in the framework.

This is a bit of a kludge, catching exceptions and re-raising them, but it is a less invasive way of doing it.


I added some examples of things to test in the ticket.

rob
>From 461ce7dfa9d2d4a473d949d4f5ec523ba77b3dd7 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 16 Mar 2012 13:30:59 -0400
Subject: [PATCH] Use a consistent parameter name in errors, defaulting to
 cli_name.

For general command-line errors we want to use the cli_name on output.
The exception is when using *attr, we want to return that attribute name
in the exception.

https://fedorahosted.org/freeipa/ticket/1418
---
 ipalib/parameters.py       |   35 +++++++++++++++++++++--------------
 ipalib/plugins/baseldap.py |   11 +++++++++--
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 48155daf9bc69f6b50f390737b8b69a2c84e12a2..94f11d912c010fb644d9683d7630a2d753f06371 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -563,6 +563,18 @@ class Param(ReadOnly):
             self.validate(value, supplied=self.name in kw)
         return value
 
+    def get_param_name(self):
+        """
+        Return the right name of an attribute depending on usage.
+
+        Normally errors should use cli_name, our "friendly" name. When
+        using the API directly or *attr return the real name.
+        """
+        name = self.cli_name
+        if not name:
+            name = self.name
+        return name
+
     def kw(self):
         """
         Iterate through ``(key,value)`` for all kwargs passed to constructor.
@@ -861,11 +873,8 @@ class Param(ReadOnly):
         for rule in self.all_rules:
             error = rule(ugettext, value)
             if error is not None:
-                name = self.cli_name
-                if not name:
-                    name = self.name
                 raise ValidationError(
-                    name=name,
+                    name=self.get_param_name(),
                     value=value,
                     index=index,
                     error=error,
@@ -1175,7 +1184,7 @@ class Int(Number):
                 return int(value)
             except ValueError:
                 pass
-        raise ConversionError(name=self.name, index=index,
+        raise ConversionError(name=self.get_param_name(), index=index,
             error=ugettext(self.type_error),
         )
 
@@ -1218,11 +1227,8 @@ class Int(Number):
         for rule in self.all_rules:
             error = rule(ugettext, value)
             if error is not None:
-                name = self.cli_name
-                if not name:
-                    name = self.name
                 raise ValidationError(
-                    name=name,
+                    name=self.get_param_name(),
                     value=value,
                     index=index,
                     error=error,
@@ -1309,7 +1315,7 @@ class Decimal(Number):
             try:
                 value = decimal.Decimal(value)
             except Exception, e:
-                raise ConversionError(name=self.name, index=index,
+                raise ConversionError(name=self.get_param_name(), index=index,
                                       error=unicode(e))
 
         if isinstance(value, decimal.Decimal):
@@ -1449,7 +1455,7 @@ class Bytes(Data):
             try:
                 value = base64.b64decode(value)
             except TypeError:
-                raise ConversionError(name=self.name, index=index, error=self.type_error)
+                raise ConversionError(name=self.get_param_name(), index=index, error=self.type_error)
         return super(Bytes, self)._convert_scalar(value, index)
 
 
@@ -1548,7 +1554,8 @@ class IA5Str(Str):
         if isinstance(value, basestring):
             for i in xrange(len(value)):
                 if ord(value[i]) > 127:
-                    raise ConversionError(name=self.name, index=index,
+                    raise ConversionError(name=self.get_param_name(),
+                        index=index,
                         error=_('The character \'%(char)r\' is not allowed.') %
                             dict(char=value[i],)
                     )
@@ -1832,10 +1839,10 @@ class AccessTime(Str):
         try:
             self._check(value)
         except ValueError, e:
-            raise ValidationError(name=self.cli_name, error=e.args[0])
+            raise ValidationError(name=self.get_param_name(), error=e.args[0])
         except IndexError:
             raise ValidationError(
-                name=self.cli_name, error='incomplete time value'
+                name=self.get_param_name(), error='incomplete time value'
             )
         return None
 
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index f3c89a0fc5e3f00ed7f132dbff2510d89bc7370d..85a976c194c02397c7afce597a9da62bd9b90a90 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -891,9 +891,16 @@ last, after all sets and adds."""),
                     else:
                         raise errors.OnlyOneValueAllowed(attr=attr)
                 # validate and convert params
-                entry_attrs[attr] = self.obj.params[attr](entry_attrs[attr])
+                try:
+                    entry_attrs[attr] = self.obj.params[attr](entry_attrs[attr])
+                except errors.ValidationError, err:
+                    (name, error) = err.strerror.split(':')
+                    raise errors.ValidationError(name=attr, error=error)
+                except errors.ConversionError, err:
+                    (name, error) = err.strerror.split(':')
+                    raise errors.ConversionError(name=attr, error=error)
             else:
-                # unknown attribute: remove duplicite and invalid values
+                # unknown attribute: remove duplicate and invalid values
                 entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val]))
                 if not entry_attrs[attr]:
                     entry_attrs[attr] = None
-- 
1.7.6

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

Reply via email to