New Param: implemented a base Param._convert_scalar() method; added Param.type_error attribute for ConversionError message

4 files changed, 61 insertions, 22 deletions

diff --git a/ipalib/errors2.py b/ipalib/errors2.py
index 81b1fb2ed..4c8acd5d8 100644
--- a/ipalib/errors2.py
+++ b/ipalib/errors2.py
@@ -481,9 +481,17 @@ class RequirementError(InvocationError):
 class ConversionError(InvocationError):
     """
     **3006** Raised when parameter value can't be converted to correct type.
+
+    For example:
+
+    >>> raise ConversionError(name='age', error='must be an integer')
+    Traceback (most recent call last):
+      ...
+    ConversionError: invalid 'age': must be an integer
     """
 
     errno = 3006
+    format = _('invalid %(name)r: %(error)s')
 
 
 class ValidationError(InvocationError):
diff --git a/ipalib/parameter.py b/ipalib/parameter.py
index dbf752980..3ec7579a7 100644
--- a/ipalib/parameter.py
+++ b/ipalib/parameter.py
@@ -25,7 +25,7 @@ from types import NoneType
 from util import make_repr
 from request import ugettext
 from plugable import ReadOnly, lock, check_name
-from errors2 import RequirementError, ValidationError
+from errors2 import ConversionError, RequirementError, ValidationError
 from constants import NULLS, TYPE_ERROR, CALLABLE_ERROR
 
 
@@ -189,6 +189,13 @@ def parse_param_spec(spec):
     return (spec, dict(required=True, multivalue=False))
 
 
+__messages = set()
+
+def _(message):
+    __messages.add(message)
+    return message
+
+
 class Param(ReadOnly):
     """
     Base class for all parameters.
@@ -199,6 +206,9 @@ class Param(ReadOnly):
     # (direct) subclass must *always* override this class attribute:
     type = NoneType  # Ouch, this wont be very useful in the real world!
 
+    # Subclasses should override this with something more specific:
+    type_error = _('incorrect type')
+
     kwargs = (
         ('cli_name', str, None),
         ('label', callable, None),
@@ -436,10 +446,12 @@ class Param(ReadOnly):
 
     def _convert_scalar(self, value, index=None):
         """
-        Implement in subclass.
+        Convert a single scalar value.
         """
-        raise NotImplementedError(
-            '%s.%s()' % (self.__class__.__name__, '_convert_scalar')
+        if type(value) is self.type:
+            return value
+        raise ConversionError(name=self.name, index=index,
+            error=ugettext(self.type_error),
         )
 
     def validate(self, value):
@@ -602,10 +614,11 @@ class Bool(Param):
 
 class Int(Param):
     """
-
+    A parameter for integer values (stored in Python ``int`` type).
     """
 
     type = int
+    type_error = _('must be an integer')
 
 
 class Float(Param):
@@ -618,7 +631,7 @@ class Float(Param):
 
 class Bytes(Param):
     """
-    A parameter for binary data.
+    A parameter for binary data (stored in Python ``str`` type).
     """
 
     type = str
@@ -664,12 +677,6 @@ class Bytes(Param):
                         self.nice, self.minlength)
                 )
 
-    def _convert_scalar(self, value, index=None):
-        """
-        Implement in subclass.
-        """
-        return value
-
     def _rule_minlength(self, _, value):
         """
         Check minlength constraint.
@@ -703,7 +710,7 @@ class Bytes(Param):
 
 class Str(Bytes):
     """
-    A parameter for character (textual) data.
+    A parameter for character data (stored in Python ``unicode`` type).
     """
 
     type = unicode
@@ -756,8 +763,8 @@ def create_param(spec):
 
     This function allows you to create `Str` parameters (the most common) from
     a convenient shorthand that defines the parameter name, whether it is
-    required, and whether it is multivalue.  (For a definition shorthand
-    syntax, see the `parse_param_spec()` function.)
+    required, and whether it is multivalue.  (For the definition of the
+    shorthand syntax, see the `parse_param_spec()` function.)
 
     If ``spec`` is an ``str`` instance, it will be used to create a new `Str`
     parameter, which will be returned.  For example:
diff --git a/tests/test_ipalib/test_parameter.py b/tests/test_ipalib/test_parameter.py
index 3a0bd0c1e..27d348b99 100644
--- a/tests/test_ipalib/test_parameter.py
+++ b/tests/test_ipalib/test_parameter.py
@@ -22,6 +22,7 @@ Test the `ipalib.parameter` module.
 """
 
 from types import NoneType
+from inspect import isclass
 from tests.util import raises, ClassChecker, read_only
 from tests.util import dummy_ugettext, assert_equal
 from tests.data import binary_bytes, utf8_bytes, unicode_str
@@ -315,14 +316,14 @@ class test_Param(ClassChecker):
         """
         Test the `ipalib.parameter.Param._convert_scalar` method.
         """
+        dummy = dummy_ugettext()
+
+        # Test with correct type:
         o = self.cls('my_param')
-        e = raises(NotImplementedError, o._convert_scalar, 'some value')
-        assert str(e) == 'Param._convert_scalar()'
-        class Subclass(self.cls):
-            pass
-        o = Subclass('my_param')
-        e = raises(NotImplementedError, o._convert_scalar, 'some value')
-        assert str(e) == 'Subclass._convert_scalar()'
+        assert o._convert_scalar(None) is None
+        assert dummy.called() is False
+        # Test with incorrect type
+        e = raises(errors2.ConversionError, o._convert_scalar, 'hello', index=17)
 
     def test_validate(self):
         """
@@ -782,3 +783,17 @@ def test_create_param():
         e = raises(TypeError, f, spec)
         assert str(e) == \
             TYPE_ERROR % ('spec', (str, parameter.Param), spec, type(spec))
+
+
+def test_messages():
+    """
+    Test module level message in `ipalib.parameter`.
+    """
+    for name in dir(parameter):
+        if name.startswith('_'):
+            continue
+        attr = getattr(parameter, name)
+        if not (isclass(attr) and issubclass(attr, parameter.Param)):
+            continue
+        assert type(attr.type_error) is str
+        assert attr.type_error in parameter.__messages
diff --git a/tests/util.py b/tests/util.py
index c1480840d..af4c23934 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -28,6 +28,7 @@ import tempfile
 import shutil
 import ipalib
 from ipalib.plugable import Plugin
+from ipalib.request import context
 
 
 
@@ -202,6 +203,14 @@ class ClassChecker(object):
             'get_subcls()'
         )
 
+    def tearDown(self):
+        """
+        nose tear-down fixture.
+        """
+        for name in ('ugettext', 'ungettext'):
+            if hasattr(context, name):
+                delattr(context, name)
+