On 10/07/2013 11:28 PM, Nathaniel McCallum wrote:
On Mon, 2013-10-07 at 13:22 +0200, Petr Viktorin wrote:
On 10/04/2013 07:33 PM, Nathaniel McCallum wrote:
This patch is preparatory for the OTP CLI patch.
> + def _convert_scalar(self, value, index=None):
> + return Int._convert_scalar(self, value, index=index)
That won't work. In Python 2 unbound methods (such as
Int._validate_scalar) must be passed the correct type as self; passing
an IntEnum instance like this will raise a TypeError.
You'll need to either use multiple inheritance (if you feel the
framework isn't complex enough), or make a convert_int function, and
then in both Int and IntEnum just call it and handle ValueError.
For validate_scalar it would probably be best to extend
Param._validate_scalar to allow the class to define extra allowed types,
and get rid of the reimplementation in Int.
Fixed.
This works, but I do have some comments.
I'd prefer if the framework changes and IntEnum addition were in
separate patches.
I find the usage of _get_types() a bit convoluted, especially when you
need to get the "canonical" type. I've taken the liberty to do this with
an `allowed_types` attribute, which I think is easier to use, and also
doesn't break the API.
Would you agree with these changes?
I've added tests for IntEnum, as a combination of StrEnum and Int tests.
Do they look OK?
--
Petr³
From b475201139705f6f166aa6354e0685b0a272b00f Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Allow multiple types in Param type validation
Int already needed to take both int and long. This makes the functionality
available for all Param classes.
---
ipalib/parameters.py | 98 +++++++++++++--------------------
ipatests/test_ipalib/test_parameters.py | 3 +-
2 files changed, 41 insertions(+), 60 deletions(-)
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 79b9062bb52c639ce72bc7d7fae19e7fba100e51..6ac6cb278b61adb0b039e0293a2696846debc24c 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -363,7 +363,9 @@ class Param(ReadOnly):
# This is a dummy type so that most of the functionality of Param can be
# unit tested directly without always creating a subclass; however, a real
- # (direct) subclass must *always* override this class attribute:
+ # (direct) subclass must *always* override this class attribute.
+ # If multiple types are permitted, set `type` to the canonical type and
+ # `allowed_types` to a tuple of all allowed types.
type = NoneType # Ouch, this wont be very useful in the real world!
# Subclasses should override this with something more specific:
@@ -400,6 +402,11 @@ class Param(ReadOnly):
# ('default', self.type, None),
)
+ @property
+ def allowed_types(self):
+ """The allowed datatypes for this Param"""
+ return (self.type,)
+
def __init__(self, name, *rules, **kw):
# We keep these values to use in __repr__():
self.param_spec = name
@@ -415,7 +422,7 @@ def __init__(self, name, *rules, **kw):
self.nice = '%s(%r)' % (self.__class__.__name__, self.param_spec)
# Add 'default' to self.kwargs and makes sure no unknown kw were given:
- assert type(self.type) is type
+ assert all(type(t) is type for t in self.allowed_types)
if kw.get('multivalue', True):
self.kwargs += (('default', tuple, None),)
else:
@@ -782,7 +789,7 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) is self.type:
+ if type(value) in self.allowed_types:
return value
raise ConversionError(name=self.name, index=index,
error=ugettext(self.type_error),
@@ -816,7 +823,7 @@ def validate(self, value, context=None, supplied=None):
self._validate_scalar(value)
def _validate_scalar(self, value, index=None):
- if type(value) is not self.type:
+ if type(value) not in self.allowed_types:
raise TypeError(
TYPE_ERROR % (self.name, self.type, value, type(value))
)
@@ -942,7 +949,7 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) is self.type:
+ if type(value) in self.allowed_types:
return value
if isinstance(value, basestring):
value = value.lower()
@@ -1009,7 +1016,7 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) is self.type:
+ if type(value) in self.allowed_types:
return value
if type(value) in (unicode, int, long, float):
try:
@@ -1030,13 +1037,29 @@ class Int(Number):
"""
type = int
+ allowed_types = int, long
type_error = _('must be an integer')
kwargs = Param.kwargs + (
('minvalue', (int, long), int(MININT)),
('maxvalue', (int, long), int(MAXINT)),
)
+ @staticmethod
+ def convert_int(value):
+ if type(value) in Int.allowed_types:
+ return value
+
+ if type(value) is float:
+ return int(value)
+
+ if type(value) is unicode:
+ if u'.' in value:
+ return int(float(value))
+ return int(value, 0)
+
+ raise ValueError(value)
+
def __init__(self, name, *rules, **kw):
super(Int, self).__init__(name, *rules, **kw)
@@ -1050,30 +1073,12 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) in (int, long):
- return value
- if type(value) is unicode:
- # permit floating point strings
- if value.find(u'.') >= 0:
- try:
- return int(float(value))
- except ValueError:
- pass
- else:
- try:
- # 2nd arg is radix base, 2nd arg only accepted for strings.
- # Zero means determine radix base from prefix (e.g. 0x for hex)
- return int(value, 0)
- except ValueError:
- pass
- if type(value) is float:
- try:
- return int(value)
- except ValueError:
- pass
- raise ConversionError(name=self.get_param_name(), index=index,
- error=ugettext(self.type_error),
- )
+ try:
+ return Int.convert_int(value)
+ except ValueError:
+ raise ConversionError(name=self.get_param_name(),
+ index=index,
+ error=ugettext(self.type_error))
def _rule_minvalue(self, _, value):
"""
@@ -1095,31 +1100,6 @@ def _rule_maxvalue(self, _, value):
maxvalue=self.maxvalue,
)
- def _validate_scalar(self, value, index=None):
- """
- This duplicates _validate_scalar in the Param class with
- the exception that it allows both int and long types. The
- min/max rules handle size enforcement.
- """
- if type(value) not in (int, long):
- raise TypeError(
- TYPE_ERROR % (self.name, self.type, value, type(value))
- )
- if index is not None and type(index) is not int:
- raise TypeError(
- TYPE_ERROR % ('index', int, index, type(index))
- )
- for rule in self.all_rules:
- error = rule(ugettext, value)
- if error is not None:
- raise ValidationError(
- name=self.get_param_name(),
- value=value,
- index=index,
- error=error,
- rule=rule,
- )
-
class Decimal(Number):
"""
@@ -1315,7 +1295,7 @@ def _rule_pattern(self, _, value):
"""
Check pattern (regex) contraint.
"""
- assert type(value) is self.type
+ assert type(value) in self.allowed_types
if self.re.match(value) is None:
if self.re_errmsg:
return self.re_errmsg % dict(pattern=self.pattern,)
@@ -1418,7 +1398,7 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) is self.type:
+ if type(value) in self.allowed_types:
return value
if type(value) in (int, long, float, decimal.Decimal):
return self.type(value)
@@ -1522,7 +1502,7 @@ class Enum(Param):
def __init__(self, name, *rules, **kw):
super(Enum, self).__init__(name, *rules, **kw)
for (i, v) in enumerate(self.values):
- if type(v) is not self.type:
+ if type(v) not in self.allowed_types:
n = '%s values[%d]' % (self.nice, i)
raise TypeError(
TYPE_ERROR % (n, self.type, v, type(v))
@@ -1789,7 +1769,7 @@ def _convert_scalar(self, value, index=None):
"""
Convert a single scalar value.
"""
- if type(value) is self.type:
+ if type(value) in self.allowed_types:
return value
try:
diff --git a/ipatests/test_ipalib/test_parameters.py b/ipatests/test_ipalib/test_parameters.py
index 71acfce71b52c24a4bb80b846d9d2f3e3313574b..22c7b7355d60570aa03285562f0b9e6ddf4932d6 100644
--- a/ipatests/test_ipalib/test_parameters.py
+++ b/ipatests/test_ipalib/test_parameters.py
@@ -1173,7 +1173,8 @@ def test_init(self):
"""
# Test with no kwargs:
o = self.cls('my_number')
- assert o.type is int
+ assert o.type == int
+ assert o.allowed_types == (int, long)
assert isinstance(o, parameters.Int)
assert o.minvalue == int(MININT)
assert o.maxvalue == int(MAXINT)
--
1.8.3.1
From 5cedc953a92cc9379d41d1fefca733c8c647c748 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Mon, 30 Sep 2013 12:45:37 -0400
Subject: [PATCH] Add IntEnum parameter to ipalib
---
ipalib/__init__.py | 2 +-
ipalib/parameters.py | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index d822ba5956d6afb6ef6d88063f8359926e47016b..ab89ab77ec94603d242e56436021c9b6ed8663cb 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -886,7 +886,7 @@ class my_command(Command):
from frontend import Object, Method, Property
from crud import Create, Retrieve, Update, Delete, Search
from parameters import DefaultFrom, Bool, Flag, Int, Decimal, Bytes, Str, IA5Str, Password, DNParam, DeprecatedParam
-from parameters import BytesEnum, StrEnum, AccessTime, File
+from parameters import BytesEnum, StrEnum, IntEnum, AccessTime, File
from errors import SkipPluginModule
from text import _, ngettext, GettextFactory, NGettextFactory
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 6ac6cb278b61adb0b039e0293a2696846debc24c..6e342f96567f9e9c5dbe26600dd23a26345e67e5 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1546,6 +1546,27 @@ class StrEnum(Enum):
type = unicode
+class IntEnum(Enum):
+ """
+ Enumerable for integer data (stored in the ``int`` type).
+ """
+
+ type = int
+ types = int, long
+ type_error = Int.type_error
+
+ def _convert_scalar(self, value, index=None):
+ """
+ Convert a single scalar value.
+ """
+ try:
+ return Int.convert_int(value)
+ except ValueError:
+ raise ConversionError(name=self.get_param_name(),
+ index=index,
+ error=ugettext(self.type_error))
+
+
class Any(Param):
"""
A parameter capable of holding values of any type. For internal use only.
--
1.8.3.1
From 97e10e66e07f148306404e723321cb395aa4bbd6 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 7 Oct 2013 11:00:15 +0200
Subject: [PATCH] Add tests for the IntEnum class
The StrEnum and Int tests are restructured to allow sharing the tests.
Individual *Enum tests are separated into methods.
---
ipatests/test_ipalib/test_parameters.py | 154 +++++++++++++++++++++-----------
1 file changed, 100 insertions(+), 54 deletions(-)
diff --git a/ipatests/test_ipalib/test_parameters.py b/ipatests/test_ipalib/test_parameters.py
index 22c7b7355d60570aa03285562f0b9e6ddf4932d6..d1956beded9ad07a408db0e6fc230bb6c3497e43 100644
--- a/ipatests/test_ipalib/test_parameters.py
+++ b/ipatests/test_ipalib/test_parameters.py
@@ -1072,76 +1072,140 @@ def test_convert_scalar(self):
assert o._convert_scalar(u'one') == u'one'
-class test_StrEnum(ClassChecker):
+class EnumChecker(ClassChecker):
"""
- Test the `ipalib.parameters.StrEnum` class.
+ Test *Enum classes.
"""
_cls = parameters.StrEnum
def test_init(self):
- """
- Test the `ipalib.parameters.StrEnum.__init__` method.
- """
- values = (u'Hello', u'naughty', u'nurse!')
- o = self.cls('my_strenum', values=values)
- assert o.type is unicode
+ """Test the `__init__` method"""
+ values = self._test_values
+ o = self.cls(self._name, values=values)
+ assert o.type is self._datatype
assert o.values is values
assert o.class_rules == (o._rule_values,)
assert o.rules == tuple()
assert o.all_rules == (o._rule_values,)
- badvalues = (u'Hello', 'naughty', u'nurse!')
+ def test_bad_types(self):
+ """Test failure with incorrect types"""
+ badvalues = self._bad_type_values
e = raises(TypeError, self.cls, 'my_enum', values=badvalues)
assert str(e) == TYPE_ERROR % (
- "StrEnum('my_enum') values[1]", unicode, 'naughty', str
- )
+ "%s('my_enum') values[1]" % self._cls.__name__,
+ self._datatype, badvalues[1], self._bad_type)
- # Test that ValueError is raised when list of values is empty
+ def test_empty(self):
+ """Test that ValueError is raised when list of values is empty"""
badvalues = tuple()
e = raises(ValueError, self.cls, 'empty_enum', values=badvalues)
- assert_equal(str(e), "StrEnum('empty_enum'): list of values must not "
- "be empty")
+ assert_equal(str(e), "%s('empty_enum'): list of values must not "
+ "be empty" % self._cls.__name__)
def test_rules_values(self):
- """
- Test the `ipalib.parameters.StrEnum._rule_values` method.
- """
- values = (u'Hello', u'naughty', u'nurse!')
- o = self.cls('my_enum', values=values)
- rule = o._rule_values
- translation = u"values='Hello', 'naughty', 'nurse!'"
- dummy = dummy_ugettext(translation)
+ """Test the `_rule_values` method"""
- # Test with passing values:
- for v in values:
+ def test_rules_with_passing_rules(self):
+ """Test with passing values"""
+ o = self.cls('my_enum', values=self._test_values)
+ rule = o._rule_values
+ dummy = dummy_ugettext(self._translation)
+ for v in self._test_values:
assert rule(dummy, v) is None
assert dummy.called() is False
- # Test with failing values:
- for val in (u'Howdy', u'quiet', u'library!'):
+ def test_rules_with_failing_rules(self):
+ """Test with failing values"""
+ o = self.cls('my_enum', values=self._test_values)
+ rule = o._rule_values
+ dummy = dummy_ugettext(self._translation)
+ for val in self._bad_values:
assert_equal(
rule(dummy, val),
- translation % dict(values=values),
+ self._translation % dict(values=self._test_values),
)
assert_equal(dummy.message, "must be one of %(values)s")
dummy.reset()
- # test a special case when we have just one allowed value
- values = (u'Hello', )
+ def test_one_value(self):
+ """test a special case when we have just one allowed value"""
+ values = (self._test_values[0], )
o = self.cls('my_enum', values=values)
rule = o._rule_values
- translation = u"value='Hello'"
- dummy = dummy_ugettext(translation)
+ dummy = dummy_ugettext(self._single_value_translation)
- for val in (u'Howdy', u'quiet', u'library!'):
+ for val in self._bad_values:
assert_equal(
rule(dummy, val),
- translation % dict(values=values),
+ self._single_value_translation % dict(values=values),
)
assert_equal(dummy.message, "must be '%(value)s'")
dummy.reset()
+class test_StrEnum(EnumChecker):
+ """
+ Test the `ipalib.parameters.StrEnum` class.
+ """
+ _cls = parameters.StrEnum
+ _name = 'my_strenum'
+ _datatype = unicode
+ _test_values = u'Hello', u'naughty', u'nurse!'
+ _bad_type_values = u'Hello', 'naughty', u'nurse!'
+ _bad_type = str
+ _translation = u"values='Hello', 'naughty', 'nurse!'"
+ _bad_values = u'Howdy', u'quiet', u'library!'
+ _single_value_translation = u"value='Hello'"
+
+
+def check_int_scalar_conversions(o):
+ """
+ Assure radix prefixes work, str objects fail,
+ floats (native & string) are truncated,
+ large magnitude values are promoted to long,
+ empty strings & invalid numerical representations fail
+ """
+ # Assure invalid inputs raise error
+ for bad in ['hello', u'hello', True, None, u'', u'.', 8j, ()]:
+ e = raises(errors.ConversionError, o._convert_scalar, bad)
+ assert e.name == 'my_number'
+ assert e.index is None
+ # Assure large magnitude values are handled correctly
+ assert type(o._convert_scalar(sys.maxint * 2)) == long
+ assert o._convert_scalar(sys.maxint * 2) == sys.maxint * 2
+ assert o._convert_scalar(unicode(sys.maxint * 2)) == sys.maxint * 2
+ assert o._convert_scalar(long(16)) == 16
+ # Assure normal conversions produce expected result
+ assert o._convert_scalar(u'16.99') == 16
+ assert o._convert_scalar(16.99) == 16
+ assert o._convert_scalar(u'16') == 16
+ assert o._convert_scalar(u'0x10') == 16
+ assert o._convert_scalar(u'020') == 16
+
+
+class test_IntEnum(EnumChecker):
+ """
+ Test the `ipalib.parameters.IntEnum` class.
+ """
+ _cls = parameters.IntEnum
+ _name = 'my_intenum'
+ _datatype = int
+ _test_values = 1, 2, -3
+ _bad_type_values = 1, 2.0, -3
+ _bad_type = float
+ _translation = u"values=1, 2, 3"
+ _bad_values = 4, 5, -6
+ _single_value_translation = u"value=1"
+
+ def test_convert_scalar(self):
+ """
+ Test the `ipalib.parameters.IntEnum._convert_scalar` method.
+ """
+ param = self.cls('my_number', values=(1, 2, 3, 4, 5))
+ check_int_scalar_conversions(param)
+
+
class test_Number(ClassChecker):
"""
Test the `ipalib.parameters.Number` class.
@@ -1239,28 +1303,10 @@ def test_rule_maxvalue(self):
def test_convert_scalar(self):
"""
Test the `ipalib.parameters.Int._convert_scalar` method.
- Assure radix prefixes work, str objects fail,
- floats (native & string) are truncated,
- large magnitude values are promoted to long,
- empty strings & invalid numerical representations fail
"""
- o = self.cls('my_number')
- # Assure invalid inputs raise error
- for bad in ['hello', u'hello', True, None, u'', u'.']:
- e = raises(errors.ConversionError, o._convert_scalar, bad)
- assert e.name == 'my_number'
- assert e.index is None
- # Assure large magnatude values are handled correctly
- assert type(o._convert_scalar(sys.maxint*2)) == long
- assert o._convert_scalar(sys.maxint*2) == sys.maxint*2
- assert o._convert_scalar(unicode(sys.maxint*2)) == sys.maxint*2
- assert o._convert_scalar(long(16)) == 16
- # Assure normal conversions produce expected result
- assert o._convert_scalar(u'16.99') == 16
- assert o._convert_scalar(16.99) == 16
- assert o._convert_scalar(u'16') == 16
- assert o._convert_scalar(u'0x10') == 16
- assert o._convert_scalar(u'020') == 16
+ param = self.cls('my_number')
+ check_int_scalar_conversions(param)
+
class test_Decimal(ClassChecker):
"""
--
1.8.3.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
