From 50a2ab418662dc392505e1ac1488cbcc707a0d49 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Sat, 2 Jun 2018 08:35:05 +0200 Subject: [PATCH] can set valid value for an option with invalid consistency --- test/test_option_consistency.py | 62 +++++-- tiramisu/api.py | 12 +- tiramisu/config.py | 1 + tiramisu/option/ipoption.py | 4 +- tiramisu/option/netmaskoption.py | 25 ++- tiramisu/option/option.py | 302 +++++++++++++++++-------------- tiramisu/setting.py | 25 ++- tiramisu/value.py | 78 ++++---- 8 files changed, 287 insertions(+), 222 deletions(-) diff --git a/test/test_option_consistency.py b/test/test_option_consistency.py index 01d9946..56f399f 100644 --- a/test/test_option_consistency.py +++ b/test/test_option_consistency.py @@ -316,11 +316,33 @@ def test_consistency_not_equal_multi(): raises(ValueError, "api.option('b').value.set([2, 3])") -def test_consistency_not_equal_multi_default(): +def test_consistency_not_equal_multi_default1(): a = IntOption('a', '', multi=True, default=[1]) - b = IntOption('b', '', multi=True, default=[1, 2]) + b = IntOption('b', '', multi=True, default=[3, 1]) od = OptionDescription('a', '', [a, b]) - raises(ValueError, "a.impl_add_consistency('not_equal', b)") + raises(ValueError, "b.impl_add_consistency('not_equal', a)") + + +def test_consistency_not_equal_multi_default2(): + a = IntOption('a', '', multi=True, default=[1]) + b = IntOption('b', '', multi=True, default_multi=1) + od = OptionDescription('a', '', [a, b]) + #default_multi not tested now + a.impl_add_consistency('not_equal', b) + + +def test_consistency_not_equal_master_default(): + a = IntOption('a', '', multi=True, default=[2, 1]) + b = IntOption('b', '', multi=True, default_multi=1) + od = MasterSlaves('a', '', [a, b]) + a.impl_add_consistency('not_equal', b) + od2 = OptionDescription('a', '', [od]) + api = getapi(Config(od2)) + # default_multi not tested + raises(ValueError, "api.option('a.b', 0).value.get()") + api.option('a.b', 0).value.set(3) + api.option('a.b', 1).value.set(3) + assert api.option('a.b', 1).value.get() == 3 def test_consistency_not_equal_multi_default_modif(): @@ -417,24 +439,26 @@ def test_consistency_ip_in_network(): assert len(w) == 1 -def test_consistency_ip_in_network_len_error(): - a = NetworkOption('a', '') - b = NetmaskOption('b', '') - c = IPOption('c', '') - od = OptionDescription('od', '', [a, b, c]) - raises(ConfigError, "c.impl_add_consistency('in_network', a)") +# needs 3 arguments, not 2 +#def test_consistency_ip_in_network_len_error(): +# a = NetworkOption('a', '') +# b = NetmaskOption('b', '') +# c = IPOption('c', '') +# od = OptionDescription('od', '', [a, b, c]) +# raises(ConfigError, "c.impl_add_consistency('in_network', a)") -def test_consistency_ip_netmask_network_error(): - a = IPOption('a', '') - b = NetworkOption('b', '') - c = NetmaskOption('c', '') - od = OptionDescription('od', '', [a, b, c]) - c.impl_add_consistency('ip_netmask', a, b) - api = getapi(Config(od)) - api.option('a').value.set('192.168.1.1') - api.option('b').value.set('192.168.1.0') - raises(ConfigError, "api.option('c').value.set('255.255.255.0')") +# needs 2 arguments, not 3 +#def test_consistency_ip_netmask_network_error(): +# a = IPOption('a', '') +# b = NetworkOption('b', '') +# c = NetmaskOption('c', '') +# od = OptionDescription('od', '', [a, b, c]) +# c.impl_add_consistency('ip_netmask', a, b) +# api = getapi(Config(od)) +# api.option('a').value.set('192.168.1.1') +# api.option('b').value.set('192.168.1.0') +# raises(ConfigError, "api.option('c').value.set('255.255.255.0')") def test_consistency_ip_netmask_error_multi(): diff --git a/tiramisu/api.py b/tiramisu/api.py index 2283042..d788d10 100644 --- a/tiramisu/api.py +++ b/tiramisu/api.py @@ -30,10 +30,10 @@ from .option import ChoiceOption TIRAMISU_VERSION = 3 -try: - from .value import Multi -except: - Multi = list +#try: +# from .value import Multi +#except: +# Multi = list COUNT_TIME = False @@ -532,8 +532,8 @@ class TiramisuOptionValue(CommonTiramisuOption): value = self.subconfig.getattr(self._name, self.index, self.config_bag) - if isinstance(value, Multi): - value = list(value) + #if isinstance(value, Multi): + # value = list(value) return value @count diff --git a/tiramisu/config.py b/tiramisu/config.py index 9154eb1..94b5501 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -75,6 +75,7 @@ class SubConfig(object): masterpath = master.impl_getname() mconfig_bag = config_bag.copy('nooption') mconfig_bag.option = master + mconfig_bag.validate = False value = self.getattr(masterpath, None, mconfig_bag) diff --git a/tiramisu/option/ipoption.py b/tiramisu/option/ipoption.py index 7b56721..087e166 100644 --- a/tiramisu/option/ipoption.py +++ b/tiramisu/option/ipoption.py @@ -102,9 +102,7 @@ class IPOption(Option): opts, vals, warnings_only): - if len(vals) != 3: - raise ConfigError(_('invalid len for vals')) - if None in vals: + if len(vals) != 3 or None in vals: return ip, network, netmask = vals if IP(ip) not in IP('{0}/{1}'.format(network, diff --git a/tiramisu/option/netmaskoption.py b/tiramisu/option/netmaskoption.py index 5bc047e..c66fd25 100644 --- a/tiramisu/option/netmaskoption.py +++ b/tiramisu/option/netmaskoption.py @@ -52,13 +52,15 @@ class NetmaskOption(Option): vals, warnings_only): #opts must be (netmask, network) options - if None in vals: + if len(vals) != 2 or None in vals: return - return self.__cons_netmask(opts, + return self.__cons_netmask(current_opt, + opts, vals[0], vals[1], False, - warnings_only) + warnings_only, + 'network') def _cons_ip_netmask(self, current_opt, @@ -66,20 +68,24 @@ class NetmaskOption(Option): vals, warnings_only): #opts must be (netmask, ip) options - if None in vals: + if len(vals) != 2 or None in vals: return - self.__cons_netmask(opts, + self.__cons_netmask(current_opt, + opts, vals[0], vals[1], True, - warnings_only) + warnings_only, + 'ip') def __cons_netmask(self, + current_opt, opts, val_netmask, val_ipnetwork, make_net, - warnings_only): + warnings_only, + typ): if len(opts) != 2: raise ConfigError(_('invalid len for opts')) msg = None @@ -95,7 +101,10 @@ class NetmaskOption(Option): except ValueError: if not make_net: - msg = _('with netmask "{0}" ("{1}")') + if current_opt == opts[1]: + raise ValueError(_('with netmask "{0}" ("{1}")').format(val_netmask, opts[0].impl_get_display_name())) + else: + raise ValueError(_('with {2} "{0}" ("{1}")').format(val_ipnetwork, opts[1].impl_get_display_name(), typ)) if msg is not None: raise ValueError(msg.format(val_netmask, opts[1].impl_get_display_name())) diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 43633fd..35a9d8d 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -198,14 +198,20 @@ class Option(OnlyOption): """ if current_opt is undefined: current_opt = self - - if config_bag is not undefined and \ - ((check_error is True and not 'validator' in config_bag.setting_properties) or \ - (check_error is False and not 'warnings' in config_bag.setting_properties)): + + if check_error and config_bag is not undefined and \ + not config_bag.validate: + # just to check propertieserror + self.valid_consistency(current_opt, + value, + context, + force_index, + check_error, + config_bag) return def _is_not_unique(value): - #FIXME pourquoi la longueur doit etre egal ??? + #FIXME pourquoi la longueur doit etre egal ? if check_error and self.impl_is_unique() and len(set(value)) != len(value): for idx, val in enumerate(value): if val in value[idx+1:]: @@ -303,12 +309,12 @@ class Option(OnlyOption): do_validation(val, idx) - self._valid_consistency(current_opt, - value, - context, - force_index, - check_error, - config_bag) + self.valid_consistency(current_opt, + value, + context, + force_index, + check_error, + config_bag) except ValueError as err: if debug: # pragma: no cover log.debug('do_validation: value: {0}, index: {1}:' @@ -458,13 +464,13 @@ class Option(OnlyOption): self._add_dependency(opt) opt._add_dependency(self) - def _valid_consistency(self, - option, - value, - context, - index, - check_error, - config_bag): + def valid_consistency(self, + option, + value, + context, + index, + check_error, + config_bag): if context is not undefined: descr = context.cfgimpl_get_description() # no consistency found at all @@ -494,30 +500,70 @@ class Option(OnlyOption): else: opts = all_cons_opts wopt = opts[0]() - wopt._launch_consistency(self, - func, - cons_id, - option, - value, - context, - index, - opts, - warnings_only, - transitive, - config_bag) + wopt.launch_consistency(self, + func, + cons_id, + option, + value, + context, + index, + opts, + warnings_only, + transitive, + config_bag) - def _launch_consistency(self, - current_opt, - func, - cons_id, - option, - value, - context, - index, - opts, - warnings_only, - transitive, - config_bag): + def _get_consistency_value(self, + orig_option, + current_option, + index, + cons_id, + context, + config_bag, + value, + transitive, + func): + if func in ALLOWED_CONST_LIST: + index = None + index_ = None + elif not current_option.impl_is_master_slaves('slave'): + index_ = None + else: + index_ = index + if orig_option == current_option: + # orig_option is current option + # we have already value, so use it + return value + if context is undefined: + #if no context get default value + return current_option.impl_getdefault() + #otherwise calculate value + sconfig_bag = config_bag.copy('nooption') + sconfig_bag.option = current_option + sconfig_bag.fromconsistency.append(cons_id) + sconfig_bag.force_permissive = True + path = current_option.impl_getpath(context) + opt_value = context.getattr(path, + index_, + sconfig_bag) + if index_ is None and index is not None: + if len(opt_value) <= index: + opt_value = current_option.impl_getdefault_multi() + else: + opt_value = opt_value[index] + return opt_value + + def launch_consistency(self, + current_opt, + func, + cons_id, + option, + value, + context, + index, + opts, + warnings_only, + transitive, + config_bag): """Launch consistency now :param func: function name, this name should start with _cons_ @@ -537,111 +583,93 @@ class Option(OnlyOption): :param transitive: propertyerror is transitive :type transitive: `boolean` """ - if context is not undefined: - descr = context.cfgimpl_get_description() + #if context is not undefined: + # descr = context.cfgimpl_get_description() if config_bag is not undefined and cons_id in config_bag.fromconsistency: return all_cons_vals = [] all_cons_opts = [] length = None for opt in opts: - if isinstance(opt, weakref.ReferenceType): + if isinstance(opt, weakref.ReferenceType): opt = opt() - if option == opt: - # option is current option - # we have already value, so use it - opt_value = value - elif context is undefined: - opt_value = opt.impl_getdefault() - else: - #if context, calculate value, otherwise get default value - sconfig_bag = config_bag.copy('nooption') - sconfig_bag.option = opt - sconfig_bag.fromconsistency.append(cons_id) - sconfig_bag.force_permissive = True - path = opt.impl_getpath(context) - if opt.impl_is_master_slaves('slave'): - index_ = index - else: - index_ = None - try: - opt_value = context.getattr(path, - index_, - sconfig_bag) - except PropertiesOptionError as err: - if debug: # pragma: no cover - log.debug('propertyerror in _launch_consistency: {0}'.format(err)) - if transitive: - err.set_orig_opt(option) - raise err - opt_value = None - if not option == opt and opt_value is not None and index is not None and \ - (context is undefined or \ - not opt.impl_is_master_slaves('slave')): - if len(opt_value) <= index: - opt_value = opt.impl_getdefault_multi() - else: - opt_value = opt_value[index] - - if opt_value is not None and opt.impl_is_multi() and index is None and func not in ALLOWED_CONST_LIST: - if length is not None and length != len(opt_value): - if context is undefined: - return - raise ValueError(_('unexpected length of "{}" in constency "{}", should be "{}"' - '').format(len(opt_value), - opt.impl_get_display_name(), - length)) - else: - length = len(opt_value) - is_multi = True - else: - is_multi = False - if isinstance(opt_value, list) and func in ALLOWED_CONST_LIST: - for value_ in opt_value: - if isinstance(value_, list): - for val in value_: - all_cons_vals.append((False, val)) - all_cons_opts.append(opt) - else: - all_cons_vals.append((False, value_)) - all_cons_opts.append(opt) - else: - all_cons_vals.append((is_multi, opt_value)) - all_cons_opts.append(opt) - else: + is_multi = False try: - all_values = [] - if length is None: - all_value = [] - for is_multi, values in all_cons_vals: - all_value.append(values) - all_values = [all_value] - else: - for idx in range(length): - all_value = [] - for is_multi, values in all_cons_vals: - if not is_multi: - all_value.append(values) - else: - all_value.append(values[idx]) - all_values.append(all_value) - for values in all_values: - getattr(self, func)(current_opt, - all_cons_opts, - values, - warnings_only) - except ValueError as err: - if warnings_only: - msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}' - '').format(value, - self._display_name, - current_opt.impl_get_display_name(), - err) - warnings.warn_explicit(ValueWarning(msg, weakref.ref(self)), - ValueWarning, - self.__class__.__name__, 0) - else: + opt_value = self._get_consistency_value(option, + opt, + index, + cons_id, + context, + config_bag, + value, + transitive, + func) + except PropertiesOptionError as err: + if debug: # pragma: no cover + log.debug('propertyerror in launch_consistency: {0}'.format(err)) + if transitive: + err.set_orig_opt(option) raise err + else: + if opt.impl_is_multi() and index is None and func not in ALLOWED_CONST_LIST: + len_value = len(opt_value) + if length is not None and length != len_value: + if context is undefined: + return + raise ValueError(_('unexpected length of "{}" in constency "{}", should be "{}"' + '').format(len(opt_value), + opt.impl_get_display_name(), + length)) + else: + length = len_value + is_multi = True + if isinstance(opt_value, list) and func in ALLOWED_CONST_LIST: + for value_ in opt_value: + if opt.impl_is_submulti(): + for val in value_: + all_cons_vals.append((False, val)) + all_cons_opts.append(opt) + else: + all_cons_vals.append((False, value_)) + all_cons_opts.append(opt) + else: + all_cons_vals.append((is_multi, opt_value)) + all_cons_opts.append(opt) + if config_bag is not undefined and not config_bag.validate: + return + all_values = [] + if length is None: + all_value = [] + for is_multi, values in all_cons_vals: + all_value.append(values) + all_values = [all_value] + else: + for idx in range(length): + all_value = [] + for is_multi, values in all_cons_vals: + if not is_multi: + all_value.append(values) + else: + all_value.append(values[idx]) + all_values.append(all_value) + try: + for values in all_values: + getattr(self, func)(current_opt, + all_cons_opts, + values, + warnings_only) + except ValueError as err: + if warnings_only: + msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}' + '').format(value, + self._display_name, + current_opt.impl_get_display_name(), + err) + warnings.warn_explicit(ValueWarning(msg, weakref.ref(self)), + ValueWarning, + self.__class__.__name__, 0) + else: + raise err def _cons_not_equal(self, current_opt, diff --git a/tiramisu/setting.py b/tiramisu/setting.py index 9040d20..a92869b 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -124,36 +124,53 @@ class ConfigBag(object): 'option', 'ori_option', 'properties', - 'validate', 'setting_properties', 'force_permissive', 'force_unrestraint', - 'display_warnings', 'trusted_cached_properties', 'fromconsistency', + '_validator' ) def __init__(self, config, **kwargs): self.default = {'force_permissive': False, 'force_unrestraint': False, - 'display_warnings': True, 'trusted_cached_properties': True, } self.config = config + self._validator = True self.fromconsistency = [] for key, value in kwargs.items(): if value != self.default.get(key): setattr(self, key, value) def __getattr__(self, key): - if key in ['validate', 'validate_properties']: + if key == 'validate_properties': return not self.force_unrestraint + if key == 'validate': + if self.setting_properties is not None: + return 'validator' in self.setting_properties + return self._validator if key == 'setting_properties': if self.force_unrestraint: return None self.setting_properties = self.config.cfgimpl_get_settings().get_context_properties() return self.setting_properties + if key not in self.__slots__: + raise KeyError('unknown key {}'.format(key)) return self.default.get(key) + def __setattr__(self, key, value): + if key == 'validate': + if self.setting_properties is not None: + if value is False: + self.setting_properties = frozenset(set(self.setting_properties) - {'validator'}) + else: + self.setting_properties = frozenset(set(self.setting_properties) | {'validator'}) + else: + self._validator = value + else: + super().__setattr__(key, value) + def delete(self, key): try: return self.__delattr__(key) diff --git a/tiramisu/value.py b/tiramisu/value.py index e02dfd9..508edd3 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -100,22 +100,20 @@ class Values(object): value = self.getvalue(path, index, config_bag) - #FIXME suboptimal ... # validate value - if config_bag.validate: - context = self._getcontext() - opt = config_bag.option + context = self._getcontext() + opt = config_bag.option + opt.impl_validate(value, + context=context, + force_index=index, + check_error=True, + config_bag=config_bag) + if setting_properties and 'warnings' in setting_properties: opt.impl_validate(value, context=context, force_index=index, - check_error=True, + check_error=False, config_bag=config_bag) - if config_bag.display_warnings: - opt.impl_validate(value, - context=context, - force_index=index, - check_error=False, - config_bag=config_bag) # store value in cache if not is_cached and \ setting_properties and 'cache' in setting_properties: @@ -178,9 +176,9 @@ class Values(object): #so return default value else: return value - return self._getdefaultvalue(path, - index, - config_bag) + return self.getdefaultvalue(path, + index, + config_bag) def getdefaultvalue(self, path, @@ -197,14 +195,6 @@ class Values(object): :type index: int :returns: default value """ - return self._getdefaultvalue(path, - index, - config_bag) - - def _getdefaultvalue(self, - path, - index, - config_bag): context = self._getcontext() opt = config_bag.option def _reset_cache(_value): @@ -325,9 +315,7 @@ class Values(object): context = self._getcontext() owner = context.cfgimpl_get_settings().getowner() - if config_bag.setting_properties is not None and \ - 'validator' in config_bag.setting_properties and \ - config_bag.validate: + if config_bag.validate: if index is not None or config_bag.option._has_consistencies(context): # set value to a fake config when option has dependency # validation will be complet in this case (consistency, ...) @@ -365,7 +353,7 @@ class Values(object): context = self._getcontext() settings = context.cfgimpl_get_settings() # First validate properties with this value - self_properties = config_bag.self_properties + self_properties = config_bag.properties if self_properties is None: self_properties = settings.getproperties(path, index, @@ -385,12 +373,13 @@ class Values(object): context, check_error=True, force_index=index) - # No error found so emit warnings - opt.impl_validate(value, - config_bag, - context, - check_error=False, - force_index=index) + if config_bag.setting_properties and 'warnings' in config_bag.setting_properties: + # No error found so emit warnings + opt.impl_validate(value, + config_bag, + context, + check_error=False, + force_index=index) def _setvalue(self, path, @@ -555,16 +544,16 @@ class Values(object): setting = context.cfgimpl_get_settings() hasvalue = self._p_.hasvalue(path) - if config_bag.validate and hasvalue and 'validator' in config_bag.setting_properties: + if hasvalue and config_bag.validate: fake_context = context._gen_fake_values() fake_value = fake_context.cfgimpl_get_values() sconfig_bag = config_bag.copy() sconfig_bag.validate = False fake_value.reset(path, sconfig_bag) - value = fake_value._getdefaultvalue(path, - None, - config_bag) + value = fake_value.getdefaultvalue(path, + None, + config_bag) fake_value.setvalue_validation(path, None, value, @@ -578,9 +567,9 @@ class Values(object): if 'force_store_value' in setting.getproperties(path, None, config_bag): - value = self._getdefaultvalue(path, - None, - config_bag) + value = self.getdefaultvalue(path, + None, + config_bag) self._setvalue(path, None, value, @@ -601,7 +590,7 @@ class Values(object): if self._p_.hasvalue(path, index=index): context = self._getcontext() - if config_bag.validate and 'validator' in config_bag.setting_properties: + if config_bag.validate: fake_context = context._gen_fake_values() fake_value = fake_context.cfgimpl_get_values() sconfig_bag = config_bag.copy() @@ -609,9 +598,9 @@ class Values(object): fake_value.reset_slave(path, index, sconfig_bag) - value = fake_value._getdefaultvalue(path, - index, - config_bag) + value = fake_value.getdefaultvalue(path, + index, + config_bag) fake_value.setvalue_validation(path, index, value, @@ -756,11 +745,10 @@ class Values(object): context = self._getcontext() # copy od_setting_properties = config_bag.setting_properties - {'mandatory', 'empty'} - setting_properties = set(config_bag.setting_properties) + setting_properties = set(config_bag.setting_properties) - {'warnings'} setting_properties.update(['mandatory', 'empty']) config_bag.setting_properties = frozenset(setting_properties) config_bag.force_permissive = True - config_bag.display_warnings = False descr = context.cfgimpl_get_description() return self._mandatory_warnings(context,