From 861294c0d62891dc9977e7203cc07496e35ec939 Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: Mon, 23 May 2016 13:20:27 +0200 Subject: [PATCH] parameters: remove unused ConversionError and ValidationError arguments Do not set the `value`, `index` and `rule` arguments when raising ConversionError and ValidationError. The arguments are unused and are not specified consistently accross the framework and plugins. https://fedorahosted.org/freeipa/ticket/4739 Reviewed-By: David Kupka --- ipalib/parameters.py | 79 ++++++++----------------- ipalib/plugins/dns.py | 10 ++-- ipalib/plugins/otptoken.py | 6 +- ipalib/plugins/permission.py | 2 +- ipatests/test_ipalib/test_parameters.py | 24 +------- 5 files changed, 37 insertions(+), 84 deletions(-) diff --git a/ipalib/parameters.py b/ipalib/parameters.py index 2ebd74d25..d1d065ef2 100644 --- a/ipalib/parameters.py +++ b/ipalib/parameters.py @@ -796,8 +796,7 @@ class Param(ReadOnly): if type(value) not in (tuple, list): value = (value,) values = tuple( - self._convert_scalar(v, i) - for (i, v) in enumerate(value) if not _is_null(v) + self._convert_scalar(v) for v in value if not _is_null(v) ) if len(values) == 0: return @@ -810,9 +809,7 @@ class Param(ReadOnly): """ if type(value) in self.allowed_types: return value - raise ConversionError(name=self.name, index=index, - error=ugettext(self.type_error), - ) + raise ConversionError(name=self.name, error=ugettext(self.type_error)) def validate(self, value, context=None, supplied=None): """ @@ -836,8 +833,8 @@ class Param(ReadOnly): ) if len(value) < 1: raise ValueError('value: empty tuple must be converted to None') - for (i, v) in enumerate(value): - self._validate_scalar(v, i) + for v in value: + self._validate_scalar(v) else: self._validate_scalar(value) @@ -846,20 +843,10 @@ class Param(ReadOnly): 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, - ) + raise ValidationError(name=self.get_param_name(), error=error) def get_default(self, **kw): """ @@ -975,11 +962,9 @@ class Bool(Param): if value in self.falsehoods: return False if type(value) in (tuple, list): - raise ConversionError(name=self.name, index=index, - error=ugettext(self.scalar_error)) - raise ConversionError(name=self.name, index=index, - error=ugettext(self.type_error), - ) + raise ConversionError(name=self.name, + error=ugettext(self.scalar_error)) + raise ConversionError(name=self.name, error=ugettext(self.type_error)) class Flag(Bool): @@ -1041,11 +1026,9 @@ class Number(Param): except ValueError: pass if type(value) in (tuple, list): - raise ConversionError(name=self.name, index=index, - error=ugettext(self.scalar_error)) - raise ConversionError(name=self.name, index=index, - error=ugettext(self.type_error), - ) + raise ConversionError(name=self.name, + error=ugettext(self.scalar_error)) + raise ConversionError(name=self.name, error=ugettext(self.type_error)) class Int(Number): @@ -1097,7 +1080,6 @@ class Int(Number): 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): @@ -1244,13 +1226,13 @@ class Decimal(Number): try: value = decimal.Decimal(value) except decimal.DecimalException as e: - raise ConversionError(name=self.get_param_name(), index=index, + raise ConversionError(name=self.get_param_name(), error=unicode(e)) if isinstance(value, decimal.Decimal): return self._test_and_normalize(value) - return super(Decimal, self)._convert_scalar(value, index) + return super(Decimal, self)._convert_scalar(value) def _normalize_scalar(self, value): if isinstance(value, decimal.Decimal): @@ -1386,7 +1368,7 @@ class Bytes(Data): value = base64.b64decode(value) except (TypeError, ValueError) as e: raise Base64DecodeError(reason=str(e)) - return super(Bytes, self)._convert_scalar(value, index) + return super(Bytes, self)._convert_scalar(value) class Str(Data): @@ -1426,11 +1408,9 @@ class Str(Data): if type(value) in (float, decimal.Decimal) + six.integer_types: return self.type(value) if type(value) in (tuple, list): - raise ConversionError(name=self.name, index=index, - error=ugettext(self.scalar_error)) - raise ConversionError(name=self.name, index=index, - error=ugettext(self.type_error), - ) + raise ConversionError(name=self.name, + error=ugettext(self.scalar_error)) + raise ConversionError(name=self.name, error=ugettext(self.type_error)) def _rule_noextrawhitespace(self, _, value): """ @@ -1488,11 +1468,10 @@ class IA5Str(Str): for char in value: if ord(char) > 127: raise ConversionError(name=self.get_param_name(), - index=index, error=_('The character %(char)r is not allowed.') % dict(char=char,) ) - return super(IA5Str, self)._convert_scalar(value, index) + return super(IA5Str, self)._convert_scalar(value) class Password(Str): @@ -1510,9 +1489,9 @@ class Password(Str): if isinstance(value, (tuple, list)) and len(value) == 2: (p1, p2) = value if p1 != p2: - raise PasswordMismatch(name=self.name, index=index) + raise PasswordMismatch(name=self.name) value = p1 - return super(Password, self)._convert_scalar(value, index) + return super(Password, self)._convert_scalar(value) class Enum(Param): @@ -1588,7 +1567,6 @@ class IntEnum(Enum): return Int.convert_int(value) except ValueError: raise ConversionError(name=self.get_param_name(), - index=index, error=ugettext(self.type_error)) @@ -1606,13 +1584,7 @@ class Any(Param): for rule in self.all_rules: error = rule(ugettext, value) if error is not None: - raise ValidationError( - name=self.name, - value=value, - index=index, - error=error, - rule=rule, - ) + raise ValidationError(name=self.name, error=error) class File(Str): @@ -1671,10 +1643,9 @@ class DateTime(Param): (', '.join(self.accepted_formats))) raise ConversionError(name=self.get_param_name(), - index=index, error=error) - return super(DateTime, self)._convert_scalar(value, index) + return super(DateTime, self)._convert_scalar(value) class AccessTime(Str): @@ -1870,7 +1841,7 @@ class DNParam(Param): try: dn = DN(value) except Exception as e: - raise ConversionError(name=self.get_param_name(), index=index, + raise ConversionError(name=self.get_param_name(), error=ugettext(e)) return dn @@ -1961,14 +1932,14 @@ class DNSNameParam(Param): try: validate_idna_domain(value) except ValueError as e: - raise ConversionError(name=self.get_param_name(), index=index, + raise ConversionError(name=self.get_param_name(), error=unicode(e)) value = DNSName(value) if self.only_absolute and not value.is_absolute(): value = value.make_absolute() - return super(DNSNameParam, self)._convert_scalar(value, index) + return super(DNSNameParam, self)._convert_scalar(value) def _rule_only_absolute(self, _, value): if self.only_absolute and not value.is_absolute(): diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index 079cb6d98..8a31dcfeb 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -715,7 +715,7 @@ class DNSRecord(Str): return None return tuple(values) - def _part_values_to_string(self, values, index, idna=True): + def _part_values_to_string(self, values, idna=True): self._validate_parts(values) parts = [] for v in values: @@ -753,8 +753,8 @@ class DNSRecord(Str): def _convert_scalar(self, value, index=None): if isinstance(value, (tuple, list)): - return self._part_values_to_string(value, index) - return super(DNSRecord, self)._convert_scalar(value, index) + return self._part_values_to_string(value) + return super(DNSRecord, self)._convert_scalar(value) def normalize(self, value): if self.normalizedns: @@ -1678,7 +1678,7 @@ def _create_idn_filter(cmd, ldap, *args, **options): value[i] = v continue try: - value[i] = record._part_values_to_string(parts, None) + value[i] = record._part_values_to_string(parts) except errors.ValidationError: value[i] = v @@ -1745,7 +1745,7 @@ def _records_idn_postprocess(record, **options): for (i, p) in enumerate(parts): if isinstance(part_params[i], DNSNameParam): parts[i] = DNSName(p) - rrs.append(param._part_values_to_string(parts, None, + rrs.append(param._part_values_to_string(parts, idna=options.get('raw', False))) except (errors.ValidationError, errors.ConversionError): rrs.append(dnsvalue) diff --git a/ipalib/plugins/otptoken.py b/ipalib/plugins/otptoken.py index d1a4034d9..9ea2c3726 100644 --- a/ipalib/plugins/otptoken.py +++ b/ipalib/plugins/otptoken.py @@ -97,16 +97,16 @@ class OTPTokenKey(Bytes): if isinstance(value, (tuple, list)) and len(value) == 2: (p1, p2) = value if p1 != p2: - raise PasswordMismatch(name=self.name, index=index) + raise PasswordMismatch(name=self.name) value = p1 if isinstance(value, unicode): try: value = base64.b32decode(value, True) except TypeError as e: - raise ConversionError(name=self.name, index=index, error=str(e)) + raise ConversionError(name=self.name, error=str(e)) - return super(OTPTokenKey, self)._convert_scalar(value, index) + return super(OTPTokenKey, self)._convert_scalar(value) def _convert_owner(userobj, entry_attrs, options): if 'ipatokenowner' in entry_attrs and not options.get('raw', False): diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py index 2312008fa..17a6d9da4 100644 --- a/ipalib/plugins/permission.py +++ b/ipalib/plugins/permission.py @@ -141,7 +141,7 @@ class DNOrURL(DNParam): def _convert_scalar(self, value, index=None): if isinstance(value, six.string_types) and value.startswith('ldap:///'): value = strip_ldap_prefix(value) - return super(DNOrURL, self)._convert_scalar(value, index=index) + return super(DNOrURL, self)._convert_scalar(value) def validate_type(ugettext, typestr): diff --git a/ipatests/test_ipalib/test_parameters.py b/ipatests/test_ipalib/test_parameters.py index ea6b80ca8..0968a11d5 100644 --- a/ipatests/test_ipalib/test_parameters.py +++ b/ipatests/test_ipalib/test_parameters.py @@ -450,7 +450,7 @@ class test_Param(ClassChecker): assert o._convert_scalar(None) is None assert dummy.called() is False # Test with incorrect type - e = raises(errors.ConversionError, o._convert_scalar, 'hello', index=17) + e = raises(errors.ConversionError, o._convert_scalar, 'hello') def test_validate(self): """ @@ -555,14 +555,10 @@ class test_Param(ClassChecker): # Test that TypeError is appropriately raised: e = raises(TypeError, o._validate_scalar, 0) assert str(e) == TYPE_ERROR % ('my_param', bool, 0, int) - e = raises(TypeError, o._validate_scalar, 'Hi', index=4) - assert str(e) == TYPE_ERROR % ('my_param', bool, 'Hi', str) - e = raises(TypeError, o._validate_scalar, True, index=3.0) - assert str(e) == TYPE_ERROR % ('index', int, 3.0, float) # Test with passing rule: - assert o._validate_scalar(True, index=None) is None - assert o._validate_scalar(False, index=None) is None + assert o._validate_scalar(True) is None + assert o._validate_scalar(False) is None assert okay.calls == [ (text.ugettext, True), (text.ugettext, False), @@ -575,11 +571,6 @@ class test_Param(ClassChecker): e = raises(errors.ValidationError, o._validate_scalar, True) assert e.name == 'my_param' assert e.error == u'this describes the error' - assert e.index is None - e = raises(errors.ValidationError, o._validate_scalar, False, index=2) - assert e.name == 'my_param' - assert e.error == u'this describes the error' - assert e.index == 2 assert okay.calls == [ (text.ugettext, True), (text.ugettext, False), @@ -937,17 +928,11 @@ class test_Str(ClassChecker): for value in bad: e = raises(errors.ConversionError, mthd, value) assert e.name == 'my_str' - assert e.index is None - assert_equal(unicode(e.error), u'must be Unicode text') - e = raises(errors.ConversionError, mthd, value, index=18) - assert e.name == 'my_str' - assert e.index == 18 assert_equal(unicode(e.error), u'must be Unicode text') bad = [(u'Hello',), [42.3]] for value in bad: e = raises(errors.ConversionError, mthd, value) assert e.name == 'my_str' - assert e.index is None assert_equal(unicode(e.error), u'Only one value is allowed') assert o.convert(None) is None @@ -1091,7 +1076,6 @@ class test_Password(ClassChecker): o = self.cls('my_password') e = raises(errors.PasswordMismatch, o._convert_scalar, [u'one', u'two']) assert e.name == 'my_password' - assert e.index is None assert o._convert_scalar([u'one', u'one']) == u'one' assert o._convert_scalar(u'one') == u'one' @@ -1194,7 +1178,6 @@ def check_int_scalar_conversions(o): 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.maxsize * 2)) == long assert o._convert_scalar(sys.maxsize * 2) == sys.maxsize * 2 @@ -1604,7 +1587,6 @@ class test_IA5Str(ClassChecker): for value in bad: e = raises(errors.ConversionError, mthd, value) assert e.name == 'my_str' - assert e.index is None if six.PY2: assert_equal(e.error, "The character '\\xc3' is not allowed.") else: