239: Added errors.ConversionError; started big clean up of how ValidationError is raised so it works well with multivalues

6 files changed, 80 insertions, 56 deletions

diff --git a/ipalib/errors.py b/ipalib/errors.py
index d68bac407..8ecccf2b2 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -119,6 +119,14 @@ class ValidationError(IPAError):
         IPAError.__init__(self, name, value, error)
 
 
+class ConversionError(ValidationError):
+    def __init__(self, name, value, type_, position):
+        self.type = type_
+        self.position = position
+        ValidationError.__init__(self, name, value, type_.conversion_error)
+
+
+
 class NormalizationError(ValidationError):
     def __init__(self, name, value, type):
         self.type = type
diff --git a/ipalib/ipa_types.py b/ipalib/ipa_types.py
index c120b5abf..2da8e0be8 100644
--- a/ipalib/ipa_types.py
+++ b/ipalib/ipa_types.py
@@ -58,6 +58,9 @@ class Type(ReadOnly):
         if type_ not in allowed:
             raise ValueError('not an allowed type: %r' % type_)
         self.type = type_
+        # FIXME: This should be replaced with a more user friendly message
+        # as this is what is returned to the user.
+        self.conversion_error = 'Must be a %r' % self.type
         lock(self)
 
     def __get_name(self):
@@ -73,6 +76,9 @@ class Type(ReadOnly):
         except (TypeError, ValueError):
             return None
 
+    def validate(self, value):
+        pass
+
     def __call__(self, value):
         if value is None:
             raise TypeError('value cannot be None')
@@ -102,10 +108,6 @@ class Bool(Type):
             return False
         return None
 
-    def validate(self, value):
-        if not (value is True or value is False):
-            return 'Must be %r or %r' % (self.true, self.false)
-
 
 class Int(Type):
     def __init__(self, min_value=None, max_value=None):
diff --git a/ipalib/public.py b/ipalib/public.py
index 72a08d7a0..24d416c91 100644
--- a/ipalib/public.py
+++ b/ipalib/public.py
@@ -27,7 +27,7 @@ import inspect
 import plugable
 from plugable import lock, check_name
 import errors
-from errors import check_type, check_isinstance
+from errors import check_type, check_isinstance, raise_TypeError
 import ipa_types
 
 
@@ -105,28 +105,36 @@ class Option(plugable.ReadOnly):
         self.rules = (type_.validate,) + rules
         lock(self)
 
+    def __convert_scalar(self, value, position=None):
+        if value is None:
+            raise TypeError('value cannot be None')
+        converted = self.type(value)
+        if converted is None:
+            raise errors.ConversionError(
+                self.name, value, self.type, position
+            )
+        return converted
+
     def convert(self, value):
         if self.multivalue:
             if type(value) in (tuple, list):
-                return tuple(self.type(v) for v in value)
-            return (self.type(value),)
-        return self.type(value)
+                return tuple(
+                    self.__convert_scalar(v, i) for (i, v) in enumerate(value)
+                )
+            return (self.__convert_scalar(value, 0),) # tuple
+        return self.__convert_scalar(value)
 
     def __normalize_scalar(self, value):
-        if value is None:
-            return None
         if type(value) is not self.type.type:
-            raise TypeError('need a %r; got %r' % (self.type.type, value))
+            raise_TypeError(value, self.type.type, 'value')
         return self.__normalize(value)
 
     def normalize(self, value):
         if self.__normalize is None:
             return value
         if self.multivalue:
-            if value is None:
-                return None
             if type(value) is not tuple:
-                raise TypeError('multivalue must be a tuple; got %r' % value)
+                raise_TypeError(value, tuple, 'value')
             return tuple(self.__normalize_scalar(v) for v in value)
         return self.__normalize_scalar(value)
 
@@ -137,6 +145,10 @@ class Option(plugable.ReadOnly):
                 raise errors.RuleError(self.name, value, rule, error)
 
     def validate(self, value):
+        if value is None and self.required:
+            raise errors.RequirementError(self.name)
+        else:
+            return
         if self.multivalue:
             if type(value) is not tuple:
                 raise TypeError('multivalue must be a tuple; got %r' % value)
@@ -210,13 +222,9 @@ class Command(plugable.Plugin):
 
     def validate(self, **kw):
         self.print_call('validate', kw, 1)
-        for option in self.Option():
-            value = kw.get(option.name, None)
-            if value is None:
-                if option.required:
-                    raise errors.RequirementError(option.name)
-                continue
-            option.validate(value)
+        for (key, value) in kw.iteritems():
+            if key in self.Option:
+                self.Option[key].validate(value)
 
     def execute(self, **kw):
         self.print_call('execute', kw, 1)
diff --git a/ipalib/tests/test_ipa_types.py b/ipalib/tests/test_ipa_types.py
index 360478fb1..b8e996a72 100644
--- a/ipalib/tests/test_ipa_types.py
+++ b/ipalib/tests/test_ipa_types.py
@@ -87,6 +87,11 @@ class test_Type(ClassChecker):
             e = raises(ValueError, self.cls, t)
             assert str(e) == 'not an allowed type: %r' % t
 
+    def test_validate(self):
+        o = self.cls(unicode)
+        for value in (None, u'Hello', 'Hello', 42, False):
+            assert o.validate(value) is None
+
 
 class test_Bool(ClassChecker):
     _cls = ipa_types.Bool
@@ -126,15 +131,6 @@ class test_Bool(ClassChecker):
             # value is not be converted, so None is returned
             assert o(value) is None
 
-    def test_validate(self):
-        t = 'For sure!'
-        f = 'No way!'
-        o = self.cls(true=t, false=f)
-        assert o.validate(True) is None
-        assert o.validate(False) is None
-        for value in (t, f, 0, 1, 'True', 'False', 'Yes', 'No'):
-            assert o.validate(value) == 'Must be %r or %r' % (t, f)
-
 
 class test_Int(ClassChecker):
     _cls = ipa_types.Int
diff --git a/ipalib/tests/test_public.py b/ipalib/tests/test_public.py
index 819c3d3fd..28970af98 100644
--- a/ipalib/tests/test_public.py
+++ b/ipalib/tests/test_public.py
@@ -22,6 +22,7 @@ Unit tests for `ipalib.public` module.
 """
 
 from tstutil import raises, getitem, no_set, no_del, read_only, ClassChecker
+from tstutil import check_TypeError
 from ipalib import public, plugable, errors, ipa_types
 
 
@@ -171,46 +172,41 @@ class test_Option(ClassChecker):
         doc = 'User last name'
         t = ipa_types.Unicode()
         callback = lambda value: value.lower()
-        orig = u'Hello World'
-        orig_str = str(orig)
-        norm = u'hello world'
-        tup_orig = (orig, norm, u'WONDERFUL!')
-        tup_norm = (norm, norm, u'wonderful!')
-        tup_str = (orig_str, orig)
-        all_values = (None, orig, orig_str, norm, tup_orig, tup_norm, tup_str)
-
-        ## Scenario 1: multivalue=False, normalize=None
+        values = (None, u'Hello', (u'Hello',), 'hello', ['hello'])
+
+        # Scenario 1: multivalue=False, normalize=None
         o = self.cls(name, doc, t)
-        for v in all_values:
+        for v in values:
             # When normalize=None, value is returned, no type checking:
             assert o.normalize(v) is v
 
-        ## Scenario 2: multivalue=False, normalize=callback
+        # Scenario 2: multivalue=False, normalize=callback
         o = self.cls(name, doc, t, normalize=callback)
-        assert o.normalize(None) is None
-        for v in (orig, norm):
-            assert o.normalize(v) == norm
-        for v in (orig_str, tup_orig, tup_norm, tup_str): # Not unicode
+        for v in (u'Hello', u'hello'): # Okay
+            assert o.normalize(v) == u'hello'
+        for v in [None, 'hello', (u'Hello',)]: # Not unicode
             e = raises(TypeError, o.normalize, v)
-            assert str(e) == 'need a %r; got %r' % (unicode, v)
+            assert str(e) == errors.TYPE_FORMAT % ('value', unicode, v)
+            check_TypeError(v, unicode, 'value', o.normalize, v)
 
-        ## Scenario 3: multivalue=True, normalize=None
+        # Scenario 3: multivalue=True, normalize=None
         o = self.cls(name, doc, t, multivalue=True)
-        for v in all_values:
+        for v in values:
             # When normalize=None, value is returned, no type checking:
             assert o.normalize(v) is v
 
-        ## Scenario 4: multivalue=True, normalize=callback
+        # Scenario 4: multivalue=True, normalize=callback
         o = self.cls(name, doc, t, multivalue=True, normalize=callback)
-        assert o.normalize(None) is None
-        for v in (tup_orig, tup_norm):
-            assert o.normalize(v) == tup_norm
-        for v in (orig, orig_str, norm): # Not tuple
+        for value in [(u'Hello',), (u'hello',)]: # Okay
+            assert o.normalize(value) == (u'hello',)
+        for v in (None, u'Hello', [u'hello']): # Not tuple
             e = raises(TypeError, o.normalize, v)
-            assert str(e) == 'multivalue must be a tuple; got %r' % v
-        for v in [tup_str, (norm, orig, orig_str)]: # Not unicode
+            assert str(e) == errors.TYPE_FORMAT % ('value', tuple, v)
+            check_TypeError(v, tuple, 'value', o.normalize, v)
+        for v in [('Hello',), (u'Hello', 'Hello')]: # Non unicode member
             e = raises(TypeError, o.normalize, v)
-            assert str(e) == 'need a %r; got %r' % (unicode, orig_str)
+            assert str(e) == errors.TYPE_FORMAT % ('value', unicode, 'Hello')
+            check_TypeError('Hello', unicode, 'value', o.normalize, v)
 
     def test_validate(self):
         """
diff --git a/ipalib/tests/tstutil.py b/ipalib/tests/tstutil.py
index 79e8ae383..1bf3eaab3 100644
--- a/ipalib/tests/tstutil.py
+++ b/ipalib/tests/tstutil.py
@@ -22,6 +22,7 @@ Utility functions for the unit tests.
 """
 
 import inspect
+from ipalib import errors
 
 class ExceptionNotRaised(Exception):
     """
@@ -131,3 +132,16 @@ class ClassChecker(object):
             self.__class__.__name__,
             'get_subcls()'
         )
+
+
+def check_TypeError(value, type_, name, callback, *args, **kw):
+    """
+    Tests a standard TypeError raised with `errors.raise_TypeError`.
+    """
+    e = raises(TypeError, callback, *args, **kw)
+    assert e.value == value
+    assert type(e.value) is type(value)
+    assert e.type is type_
+    assert e.name == name
+    assert type(e.name) is str
+    assert str(e) == errors.TYPE_FORMAT % (name, type_, value)