From e91568e6b600dd14bdd4e1e6d111e7e49857dc1a Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Thu, 24 Mar 2016 19:43:41 +0100 Subject: [PATCH] Validation should return exception, not raises exception Don't check force_store_value for SymLinkOption --- test/test_freeze.py | 5 ++- tiramisu/option/baseoption.py | 3 +- tiramisu/option/option.py | 61 ++++++++++++++++++--------- tiramisu/option/optiondescription.py | 2 +- tiramisu/storage/dictionary/option.py | 2 +- 5 files changed, 49 insertions(+), 24 deletions(-) diff --git a/test/test_freeze.py b/test/test_freeze.py index c23087a..eeff973 100644 --- a/test/test_freeze.py +++ b/test/test_freeze.py @@ -7,7 +7,7 @@ from py.test import raises from tiramisu.setting import owners, groups from tiramisu.option import ChoiceOption, BoolOption, IntOption, FloatOption, \ - StrOption, OptionDescription + StrOption, OptionDescription, SymLinkOption from tiramisu.config import Config from tiramisu.error import PropertiesOptionError, ConfigError @@ -28,13 +28,14 @@ def make_description_freeze(): requires=({'option': booloption, 'expected': True, 'action': 'hidden'},)) wantref2_option = BoolOption('wantref2', 'Test requires', default=False, properties=('force_store_value', 'hidden')) wantref3_option = BoolOption('wantref3', 'Test requires', default=[False], multi=True, properties=('force_store_value',)) + st2 = SymLinkOption('st2', wantref3_option) wantframework_option = BoolOption('wantframework', 'Test requires', default=False, requires=({'option': booloption, 'expected': True, 'action': 'hidden'},)) gcgroup = OptionDescription('gc', '', [gcoption, gcdummy, floatoption]) descr = OptionDescription('tiramisu', '', [gcgroup, booloption, objspaceoption, - wantref_option, wantref2_option, wantref3_option, stroption, + wantref_option, wantref2_option, wantref3_option, st2, stroption, wantframework_option, intoption, boolop]) return descr diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index 2988d83..f075f81 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -482,7 +482,8 @@ class Option(OnlyOption): if _value is None: return # option validation - err = self._validate(_value, context, current_opt) + err = self._validate(_value, context, current_opt, + returns_raise=True) if err: log.debug('do_validation: value: {0}, index: {1}, ' 'submulti_index: {2}'.format(_value, _index, diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 2f4578c..a36a1c4 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -68,7 +68,8 @@ class ChoiceOption(Option): properties=properties, warnings_only=warnings_only) - def impl_get_values(self, context, current_opt=undefined): + def impl_get_values(self, context, current_opt=undefined, + returns_raise=False): if current_opt is undefined: current_opt = self #FIXME cache? but in context... @@ -82,14 +83,21 @@ class ChoiceOption(Option): values_params = {} values = carry_out_calculation(current_opt, context=context, callback=values, - callback_params=values_params) + callback_params=values_params, + returns_raise=returns_raise) + if isinstance(values, Exception): + return values if values is not undefined and not isinstance(values, list): # pragma: optional cover raise ConfigError(_('calculated values for {0} is not a list' '').format(self.impl_getname())) return values - def _validate(self, value, context=undefined, current_opt=undefined): - values = self.impl_get_values(context, current_opt=current_opt) + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): + values = self.impl_get_values(context, current_opt=current_opt, + returns_raise=returns_raise) + if isinstance(values, Exception): + return values if values is not undefined and not value in values: # pragma: optional cover return ValueError(_('value {0} is not permitted, ' 'only {1} is allowed' @@ -101,7 +109,8 @@ class BoolOption(Option): "represents a choice between ``True`` and ``False``" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if not isinstance(value, bool): return ValueError(_('invalid boolean')) # pragma: optional cover @@ -110,7 +119,8 @@ class IntOption(Option): "represents a choice of an integer" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if not isinstance(value, int): return ValueError(_('invalid integer')) # pragma: optional cover @@ -119,7 +129,8 @@ class FloatOption(Option): "represents a choice of a floating point number" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if not isinstance(value, float): return ValueError(_('invalid float')) # pragma: optional cover @@ -128,7 +139,8 @@ class StrOption(Option): "represents the choice of a string" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if not isinstance(value, str): return ValueError(_('invalid string')) # pragma: optional cover @@ -144,7 +156,8 @@ else: __slots__ = tuple() _empty = u'' - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if not isinstance(value, unicode): return ValueError(_('invalid unicode')) # pragma: optional cover @@ -172,7 +185,8 @@ class IPOption(Option): warnings_only=warnings_only, extra=extra) - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): # sometimes an ip term starts with a zero # but this does not fit in some case, for example bind does not like it err = self._impl_valid_unicode(value) @@ -276,7 +290,8 @@ class PortOption(Option): warnings_only=warnings_only, extra=extra) - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): if isinstance(value, int): if sys.version_info[0] >= 3: # pragma: optional cover value = str(value) @@ -310,7 +325,8 @@ class NetworkOption(Option): "represents the choice of a network" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -333,7 +349,8 @@ class NetmaskOption(Option): "represents the choice of a netmask" __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -383,7 +400,8 @@ class NetmaskOption(Option): class BroadcastOption(Option): __slots__ = tuple() - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -443,7 +461,8 @@ class DomainnameOption(Option): warnings_only=warnings_only, extra=extra) - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -504,7 +523,8 @@ class EmailOption(DomainnameOption): __slots__ = tuple() username_re = re.compile(r"^[\w!#$%&'*+\-/=?^`{|}~.]+$") - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -528,7 +548,8 @@ class URLOption(DomainnameOption): proto_re = re.compile(r'(http|https)://') path_re = re.compile(r"^[A-Za-z0-9\-\._~:/\?#\[\]@!%\$&\'\(\)\*\+,;=]+$") - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -574,7 +595,8 @@ class UsernameOption(Option): #regexp build with 'man 8 adduser' informations username_re = re.compile(r"^[a-z_][a-z0-9_-]{0,30}[$a-z0-9_-]{0,1}$") - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err @@ -587,7 +609,8 @@ class FilenameOption(Option): __slots__ = tuple() path_re = re.compile(r"^[a-zA-Z0-9\-\._~/+]+$") - def _validate(self, value, context=undefined, current_opt=undefined): + def _validate(self, value, context=undefined, current_opt=undefined, + returns_raise=False): err = self._impl_valid_unicode(value) if err: return err diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index c6c29c9..2c354f4 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -124,7 +124,7 @@ class OptionDescription(BaseOption, StorageOptionDescription): else: option._set_readonly(True) is_multi = option.impl_is_multi() - if 'force_store_value' in option.impl_getproperties(): + if not isinstance(option, SymLinkOption) and 'force_store_value' in option.impl_getproperties(): force_store_values.append((subpath, option)) for func, all_cons_opts, params in option._get_consistencies(): option._valid_consistencies(all_cons_opts[1:]) diff --git a/tiramisu/storage/dictionary/option.py b/tiramisu/storage/dictionary/option.py index 1d50573..57c713c 100644 --- a/tiramisu/storage/dictionary/option.py +++ b/tiramisu/storage/dictionary/option.py @@ -97,7 +97,7 @@ class StorageBase(object): _setattr(self, '_default', default) if is_multi and default_multi is not None: - err = self._validate(default_multi) + err = self._validate(default_multi, returns_raise=True) if err: raise ValueError(_("invalid default_multi value {0} " "for option {1}: {2}").format(