From 92b469bd435d04162af14019ce7fbda2fad02356 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Thu, 15 Nov 2018 16:17:39 +0100 Subject: [PATCH] simplify tiramisu/option/baseoption.py --- test/test_option.py | 1 - test/test_option_callback.py | 14 +-- test/test_option_with_special_name.py | 11 +- test/test_slots.py | 36 ------ tiramisu/config.py | 2 +- tiramisu/error.py | 4 +- tiramisu/option/baseoption.py | 175 ++++++++++++-------------- tiramisu/option/broadcastoption.py | 3 +- tiramisu/option/dateoption.py | 3 +- tiramisu/option/domainnameoption.py | 4 +- tiramisu/option/ipoption.py | 3 +- tiramisu/option/netmaskoption.py | 7 +- tiramisu/option/networkoption.py | 3 +- tiramisu/option/option.py | 3 +- tiramisu/option/passwordoption.py | 3 +- tiramisu/option/portoption.py | 3 +- tiramisu/option/urloption.py | 3 +- 17 files changed, 115 insertions(+), 163 deletions(-) diff --git a/test/test_option.py b/test/test_option.py index 8238ecc..4e3da37 100644 --- a/test/test_option.py +++ b/test/test_option.py @@ -25,7 +25,6 @@ def test_option_valid_name(): raises(ValueError, 'IntOption(1, "")') raises(ValueError, 'IntOption("1test", "")') IntOption("test1", "") - raises(ValueError, 'IntOption("impl_test", "")') raises(ValueError, 'IntOption("_test", "")') raises(ValueError, 'IntOption(" ", "")') diff --git a/test/test_option_callback.py b/test/test_option_callback.py index 66a3d7c..afc661e 100644 --- a/test/test_option_callback.py +++ b/test/test_option_callback.py @@ -264,15 +264,15 @@ def test_param_option(): def test_callback_invalid(): - raises(ValueError, 'val1 = StrOption("val1", "", callback="string")') - raises(ValueError, 'val1 = StrOption("val1", "", callback=return_val, callback_params="string")') + raises(AssertionError, 'val1 = StrOption("val1", "", callback="string")') + raises(AssertionError, 'val1 = StrOption("val1", "", callback=return_val, callback_params="string")') val1 = StrOption('val1', "", 'val') val1 - raises(ValueError, "StrOption('val2', '', callback=return_value, callback_params={'': 'string'})") - raises(ValueError, "StrOption('val4', '', callback=return_value, callback_params={'value': (('string', False),)})") - raises(ValueError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1, 'string'),)})") - raises(ValueError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1, False, 'unknown'),)})") - raises(ValueError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1,),)})") + raises(AssertionError, "StrOption('val2', '', callback=return_value, callback_params={'': 'string'})") + raises(AssertionError, "StrOption('val4', '', callback=return_value, callback_params={'value': (('string', False),)})") + raises(AssertionError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1, 'string'),)})") + raises(AssertionError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1, False, 'unknown'),)})") + raises(AssertionError, "StrOption('val4', '', callback=return_value, callback_params={'value': ((val1,),)})") def test_callback_with_context(): diff --git a/test/test_option_with_special_name.py b/test/test_option_with_special_name.py index 70e95a9..24aef3f 100644 --- a/test/test_option_with_special_name.py +++ b/test/test_option_with_special_name.py @@ -55,9 +55,8 @@ def test_optname_shall_not_start_with_numbers(): def test_option_has_an_api_name(): - raises(ValueError, "BoolOption('cfgimpl_get_settings', 'dummy', default=False)") - raises(ValueError, "BoolOption('impl_getdoc', 'dummy', default=False)") - raises(ValueError, "BoolOption('_unvalid', 'dummy', default=False)") - raises(ValueError, "BoolOption('6unvalid', 'dummy', default=False)") - BoolOption('unvalid6', 'dummy', default=False) - BoolOption('unvalid_', 'dummy', default=False) + b = BoolOption('impl_has_dependency', 'dummy', default=True) + descr = OptionDescription('tiramisu', '', [b]) + api = Config(descr) + assert api.option('impl_has_dependency').value.get() is True + assert b.impl_has_dependency() is False diff --git a/test/test_slots.py b/test/test_slots.py index 47b049c..8e6c4f2 100644 --- a/test/test_slots.py +++ b/test/test_slots.py @@ -121,42 +121,6 @@ def test_slots_option_readonly(): raises(AttributeError, "q._requires = 'q'") -def test_slots_option_readonly_name(): - a = ChoiceOption('a', '', ('a',)) - b = BoolOption('b', '') - c = IntOption('c', '') - d = FloatOption('d', '') - e = StrOption('e', '') - f = SymLinkOption('f', c) - g = UnicodeOption('g', '') - h = IPOption('h', '') - i = PortOption('i', '') - j = NetworkOption('j', '') - k = NetmaskOption('k', '') - l = DomainnameOption('l', '') - o = DomainnameOption('o', '') - p = DomainnameOption('p', '') - q = DomainnameOption('q', '') - m = OptionDescription('m', '', [a, b, c, d, e, f, g, h, i, j, k, l, o, p, q]) - m - raises(AttributeError, "a._name = 'a'") - raises(AttributeError, "b._name = 'b'") - raises(AttributeError, "c._name = 'c'") - raises(AttributeError, "d._name = 'd'") - raises(AttributeError, "e._name = 'e'") - raises(AttributeError, "f._name = 'f'") - raises(AttributeError, "g._name = 'g'") - raises(AttributeError, "h._name = 'h'") - raises(AttributeError, "i._name = 'i'") - raises(AttributeError, "j._name = 'j'") - raises(AttributeError, "k._name = 'k'") - raises(AttributeError, "l._name = 'l'") - raises(AttributeError, "m._name = 'm'") - raises(AttributeError, "o._name = 'o'") - raises(AttributeError, "p._name = 'p'") - raises(AttributeError, "q._name = 'q'") - - #def test_slots_description(): # # __slots__ for OptionDescription should be complete for __getattr__ # slots = set() diff --git a/tiramisu/config.py b/tiramisu/config.py index c8f89c5..7555175 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -113,7 +113,7 @@ class SubConfig(object): if option_bag.path in resetted_opts: return resetted_opts.append(option_bag.path) - for woption in option_bag.option._get_dependencies(self): + for woption in option_bag.option._get_dependencies(self.cfgimpl_get_description()): option = woption() if option.impl_is_dynoptiondescription(): subpath = option.impl_getpath().rsplit('.', 1)[0] diff --git a/tiramisu/error.py b/tiramisu/error.py index 8d71bce..50a3b47 100644 --- a/tiramisu/error.py +++ b/tiramisu/error.py @@ -23,6 +23,8 @@ def display_list(lst, separator='and', add_quote=False): separator = _('and') elif separator == 'or': separator = _('or') + if isinstance(lst, tuple) or isinstance(lst, frozenset): + lst = list(lst) if len(lst) == 1: ret = lst[0] if not isinstance(ret, str): @@ -31,8 +33,6 @@ def display_list(lst, separator='and', add_quote=False): ret = '"{}"'.format(ret) return ret else: - if isinstance(lst, tuple): - lst = list(lst) lst.sort() lst_ = [] for l in lst[:-1]: diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index dd87783..c882c58 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -20,13 +20,15 @@ # ____________________________________________________________ import re from types import FunctionType +from typing import FrozenSet, Callable, Tuple, Set, Optional, Union, Any, List import weakref from inspect import signature from itertools import chain from ..i18n import _ -from ..setting import undefined +from ..setting import undefined, Settings +from ..value import Values from ..error import ConfigError, display_list from ..function import Params, ParamContext, ParamOption, ParamIndex @@ -38,18 +40,14 @@ NAME_REGEXP = re.compile(r'^[a-zA-Z][a-zA-Z\d_-]*$') def valid_name(name): - """an option's name is a str and does not start with 'impl' or 'cfgimpl' - and name is not a function name""" if not isinstance(name, str): return False - return re.match(NAME_REGEXP, name) is not None and \ - not name.startswith('impl_') and \ - not name.startswith('cfgimpl_') + return re.match(NAME_REGEXP, name) is not None #____________________________________________________________ # -class Base(object): +class Base: """Base use by all *Option* classes (Option, OptionDescription, SymLinkOption, ...) """ __slots__ = ('_name', @@ -70,13 +68,13 @@ class Base(object): ) def __init__(self, - name, - doc, + name: str, + doc: str, requires=None, properties=None, - is_multi=False): + is_multi: bool=False) -> None: if not valid_name(name): - raise ValueError(_('invalid name: "{0}" for option').format(name)) + raise ValueError(_('"{0}" is an invalid name for an option').format(name)) if requires is not None: calc_properties, requires = validate_requires_arg(self, is_multi, @@ -90,12 +88,16 @@ class Base(object): if isinstance(properties, tuple): properties = frozenset(properties) if is_multi and 'empty' not in properties: + # if option is a multi, it cannot be "empty" (None not allowed in the list) + # "empty" is removed for slave's option properties = properties | {'empty'} if not isinstance(properties, frozenset): raise TypeError(_('invalid properties type {0} for {1},' ' must be a frozenset').format(type(properties), name)) - self.validate_properties(name, calc_properties, properties) + self.validate_properties(name, + calc_properties, + properties) _setattr = object.__setattr__ _setattr(self, '_name', name) _setattr(self, '_informations', {'doc': doc}) @@ -106,15 +108,18 @@ class Base(object): if properties: _setattr(self, '_properties', properties) - def validate_properties(self, name, calc_properties, properties): + def validate_properties(self, + name: str, + calc_properties: FrozenSet[str], + properties: FrozenSet[str]) -> None: set_forbidden_properties = calc_properties & properties if set_forbidden_properties != frozenset(): raise ValueError(_('conflict: properties already set in requirement {0} for {1}' - '').format(display_list(list(set_forbidden_properties)), + '').format(display_list(set_forbidden_properties), name)) def _get_function_args(self, - function): + function: Callable) -> Tuple[Set[str], Set[str], bool, bool]: args = set() kwargs = set() positional = False @@ -131,8 +136,8 @@ class Base(object): return args, kwargs, positional, keyword def _get_parameters_args(self, - calculator_params, - add_value): + calculator_params: Optional[Params], + add_value: bool) -> Tuple[Set[str], Set[str]]: args = set() kwargs = set() # add value as first argument @@ -149,18 +154,17 @@ class Base(object): return args, kwargs def _build_calculator_params(self, - calculator, - calculator_params, - type_, - add_value=False): + calculator: Callable, + calculator_params: Optional[Params], + type_: str, + add_value: bool=False) -> Union[None, Params]: """ :add_value: add value as first argument for validator """ - if not isinstance(calculator, FunctionType): - raise ValueError(_('{0} must be a function').format(type_)) + assert isinstance(calculator, FunctionType), _('{0} must be a function').format(type_) if calculator_params is not None: - if not isinstance(calculator_params, Params): - raise ValueError(_('{0}_params must be a params').format(type_)) + assert isinstance(calculator_params, Params), _('{0}_params must be a params' + '').format(type_) for param in chain(calculator_params.args, calculator_params.kwargs.values()): if isinstance(param, ParamContext): self._has_calc_context = True @@ -229,34 +233,28 @@ class Base(object): return calculator_params def impl_has_dependency(self, - self_is_dep=True): + self_is_dep: bool=True) -> bool: if self_is_dep is True: - #if self.impl_is_master_slaves(): - # return True return getattr(self, '_has_dependency', False) - else: - return hasattr(self, '_dependencies') + return hasattr(self, '_dependencies') def _get_dependencies(self, - context): - if context: - od = context.cfgimpl_get_description() + context_od) -> Set[str]: ret = set(getattr(self, '_dependencies', STATIC_TUPLE)) - if context and hasattr(od, '_dependencies'): - return set(od._dependencies) | ret - else: - return ret + if context_od and hasattr(context_od, '_dependencies'): + # if context is set in options, add those options + return set(context_od._dependencies) | ret + return ret def _add_dependency(self, - option): + option) -> None: options = self._get_dependencies(None) options.add(weakref.ref(option)) self._dependencies = tuple(options) def _impl_set_callback(self, - callback, - callback_params=None): - + callback: Callable, + callback_params: Optional[Params]=None) -> None: if callback is None and callback_params is not None: raise ValueError(_("params defined for a callback function but " "no callback defined" @@ -276,22 +274,16 @@ class Base(object): val_call = (callback, callback_params) self._val_call = (val, val_call) - def impl_is_optiondescription(self): + def impl_is_optiondescription(self) -> bool: return False - def impl_is_dynoptiondescription(self): + def impl_is_dynoptiondescription(self) -> bool: return False - def impl_getname(self): + def impl_getname(self) -> str: return self._name - def impl_is_readonly(self): - return not isinstance(getattr(self, '_informations', dict()), dict) - - def impl_getproperties(self): - return getattr(self, '_properties', frozenset()) - - def _set_readonly(self): + def _set_readonly(self) -> None: if not self.impl_is_readonly(): _setattr = object.__setattr__ dico = self._informations @@ -305,11 +297,17 @@ class Base(object): if extra is not None: _setattr(self, '_extra', tuple([tuple(extra.keys()), tuple(extra.values())])) + def impl_is_readonly(self) -> str: + return not isinstance(getattr(self, '_informations', dict()), dict) + + def impl_getproperties(self) -> FrozenSet[str]: + return getattr(self, '_properties', frozenset()) + def _setsubdyn(self, - subdyn): + subdyn) -> None: self._subdyn = weakref.ref(subdyn) - def issubdyn(self): + def issubdyn(self) -> bool: return getattr(self, '_subdyn', None) is not None def getsubdyn(self): @@ -331,20 +329,17 @@ class Base(object): # ____________________________________________________________ # information def impl_get_information(self, - key, - default=undefined): + key: str, + default: Any=undefined) -> Any: """retrieves one information's item :param key: the item string (ex: "help") """ - def _is_string(infos): - return isinstance(infos, str) - dico = self._informations if isinstance(dico, tuple): if key in dico[0]: return dico[1][dico[0].index(key)] - elif _is_string(dico): + elif isinstance(dico, str): if key == 'doc': return dico elif isinstance(dico, dict): @@ -356,8 +351,8 @@ class Base(object): key)) def impl_set_information(self, - key, - value): + key: str, + value: Any) -> None: """updates the information's attribute (which is a dictionary) @@ -384,47 +379,33 @@ class BaseOption(Base): raise NotImplementedError() def __setattr__(self, - name, - value): + name: str, + value: Any) -> Any: """set once and only once some attributes in the option, - like `_name`. `_name` cannot be changed one the option and + like `_name`. `_name` cannot be changed once the option is pushed in the :class:`tiramisu.option.OptionDescription`. if the attribute `_readonly` is set to `True`, the option is - "frozen" (which has noting to do with the high level "freeze" + "frozen" (which has nothing to do with the high level "freeze" propertie or "read_only" property) """ - if name != '_option' and \ - not isinstance(value, tuple): - is_readonly = False - # never change _name dans _opt - if name == '_name': - if self.impl_getname() is not None: - #so _name is already set - is_readonly = True - elif name != '_readonly': - is_readonly = self.impl_is_readonly() - if is_readonly and (name != '_path' or value != getattr(self, '_path', value)): - raise AttributeError(_('"{}" ({}) object attribute "{}" is' - ' read-only').format(self.__class__.__name__, - self.impl_get_display_name(), - name)) + # never change _name in an option or attribute when object is readonly + if self.impl_is_readonly(): + raise AttributeError(_('"{}" ({}) object attribute "{}" is' + ' read-only').format(self.__class__.__name__, + self.impl_get_display_name(), + name)) super(BaseOption, self).__setattr__(name, value) - def impl_getpath(self): + def impl_getpath(self) -> str: return self._path - def impl_has_callback(self): + def impl_has_callback(self) -> bool: "to know if a callback has been defined or not" return self.impl_get_callback()[0] is not None - def _impl_valid_string(self, - value): - if not isinstance(value, str): - raise ValueError(_('invalid string')) - def impl_get_display_name(self, - dyn_name=None): + dyn_name: Base=None) -> str: name = self.impl_get_information('doc') if name is None or name == '': if dyn_name is not None: @@ -434,23 +415,23 @@ class BaseOption(Base): return name def reset_cache(self, - path, - values, - settings, - resetted_opts): + path: str, + values: Values, + settings: Settings, + resetted_opts: List[Base]) -> None: settings._p_.delcache(path) settings._pp_.delcache(path) if not self.impl_is_optiondescription(): values._p_.delcache(path) - def impl_is_symlinkoption(self): + def impl_is_symlinkoption(self) -> bool: return False -def validate_requires_arg(new_option, - multi, - requires, - name): +def validate_requires_arg(new_option: BaseOption, + multi: bool, + requires: List[dict], + name: str) -> Tuple[FrozenSet, Tuple]: """check malformed requirements and tranform dict to internal tuple diff --git a/tiramisu/option/broadcastoption.py b/tiramisu/option/broadcastoption.py index ac0b662..a708717 100644 --- a/tiramisu/option/broadcastoption.py +++ b/tiramisu/option/broadcastoption.py @@ -34,7 +34,8 @@ class BroadcastOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) if value.count('.') != 3: raise ValueError() for val in value.split('.'): diff --git a/tiramisu/option/dateoption.py b/tiramisu/option/dateoption.py index fa9c9cb..56f3e76 100644 --- a/tiramisu/option/dateoption.py +++ b/tiramisu/option/dateoption.py @@ -33,7 +33,8 @@ class DateOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) try: datetime.strptime(value, "%Y-%m-%d") except ValueError: diff --git a/tiramisu/option/domainnameoption.py b/tiramisu/option/domainnameoption.py index 35561e7..1d9e3ea 100644 --- a/tiramisu/option/domainnameoption.py +++ b/tiramisu/option/domainnameoption.py @@ -101,8 +101,6 @@ class DomainnameOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) - def _valid_length(val): if len(val) < 1: raise ValueError(_("invalid length (min 1)")) @@ -110,6 +108,8 @@ class DomainnameOption(Option): raise ValueError(_("invalid length (max {0})" "").format(part_name_length)) + if not isinstance(value, str): + raise ValueError(_('invalid string')) if self.impl_get_extra('_allow_ip') is True: try: IP('{0}/32'.format(value)) diff --git a/tiramisu/option/ipoption.py b/tiramisu/option/ipoption.py index db3bf2c..f3f5cb5 100644 --- a/tiramisu/option/ipoption.py +++ b/tiramisu/option/ipoption.py @@ -68,7 +68,8 @@ class IPOption(Option): **kwargs): # sometimes an ip term starts with a zero # but this does not fit in some case, for example bind does not like it - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) if value.count('.') != 3: raise ValueError() for val in value.split('.'): diff --git a/tiramisu/option/netmaskoption.py b/tiramisu/option/netmaskoption.py index 7f78d72..e8acd58 100644 --- a/tiramisu/option/netmaskoption.py +++ b/tiramisu/option/netmaskoption.py @@ -32,10 +32,11 @@ class NetmaskOption(Option): _display_name = _('netmask address') def _validate(self, - value, + value: str, *args, - **kwargs): - self._impl_valid_string(value) + **kwargs) -> None: + if not isinstance(value, str): + raise ValueError(_('invalid string')) if value.count('.') != 3: raise ValueError() for val in value.split('.'): diff --git a/tiramisu/option/networkoption.py b/tiramisu/option/networkoption.py index 0b39a69..f9d277a 100644 --- a/tiramisu/option/networkoption.py +++ b/tiramisu/option/networkoption.py @@ -34,7 +34,8 @@ class NetworkOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) if value.count('.') != 3: raise ValueError() for val in value.split('.'): diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 5266c2a..4583789 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -729,7 +729,8 @@ class RegexpOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) match = self._regexp.search(value) if not match: raise ValueError() diff --git a/tiramisu/option/passwordoption.py b/tiramisu/option/passwordoption.py index 9f81968..c225666 100644 --- a/tiramisu/option/passwordoption.py +++ b/tiramisu/option/passwordoption.py @@ -33,4 +33,5 @@ class PasswordOption(Option): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) diff --git a/tiramisu/option/portoption.py b/tiramisu/option/portoption.py index 3e428b1..137f398 100644 --- a/tiramisu/option/portoption.py +++ b/tiramisu/option/portoption.py @@ -102,7 +102,8 @@ class PortOption(Option): **kwargs): if isinstance(value, int): value = str(value) - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) if self.impl_get_extra('_allow_range') and ":" in str(value): value = str(value).split(':') if len(value) != 2: diff --git a/tiramisu/option/urloption.py b/tiramisu/option/urloption.py index 7f38678..7489975 100644 --- a/tiramisu/option/urloption.py +++ b/tiramisu/option/urloption.py @@ -36,7 +36,8 @@ class URLOption(DomainnameOption): value, *args, **kwargs): - self._impl_valid_string(value) + if not isinstance(value, str): + raise ValueError(_('invalid string')) match = self.proto_re.search(value) if not match: raise ValueError(_('must start with http:// or '