From 122796bd19a91b2ecdd2215169c3a5eca7292059 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Sun, 2 Sep 2018 11:55:19 +0200 Subject: [PATCH] requires for a master is a required for the masterslaves + remove cache from masterslaves too --- test/test_cache.py | 9 +-- test/test_requires.py | 56 +++++++++++++++++++ tiramisu/config.py | 4 +- tiramisu/option/__init__.py | 2 +- tiramisu/option/baseoption.py | 13 +++-- tiramisu/option/ipoption.py | 2 +- .../{masterslave.py => masterslaves.py} | 33 +++++++++-- tiramisu/option/option.py | 4 +- tiramisu/option/syndynoptiondescription.py | 3 +- 9 files changed, 101 insertions(+), 25 deletions(-) rename tiramisu/option/{masterslave.py => masterslaves.py} (84%) diff --git a/test/test_cache.py b/test/test_cache.py index 75f90be..3dcbc88 100644 --- a/test/test_cache.py +++ b/test/test_cache.py @@ -446,8 +446,7 @@ def test_cache_master_and_slaves_master(): compare(cfg._config.cfgimpl_get_values()._p_.get_cached(), {'val1.val1': {None: ([], None)}}) # cfg.option('val1.val1').value.set([undefined]) - compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}, - 'val1': {None: (set([]), None)}}) + compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}}) assert cfg._config.cfgimpl_get_values()._p_.get_cached() == {} cfg.config.dict() if TIRAMISU_VERSION == 2: @@ -467,8 +466,7 @@ def test_cache_master_and_slaves_master(): cfg.option('val1.val1').value.set([undefined, undefined]) cfg.config.dict() cfg.option('val1.val2', 1).value.set('oui') - compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}, - 'val1': {None: (set([]), None)}}) + compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}}) assert cfg._config.cfgimpl_get_values()._p_.get_cached() == {} if TIRAMISU_VERSION == 2: val1_val2_props = {None: (set([]), None), 0: (set([]), None), 1: (set([]), None)} @@ -520,8 +518,7 @@ def test_cache_master_callback(): 'val1.val2': {None: (val1_val2_props, None)}}) compare(cfg._config.cfgimpl_get_values()._p_.get_cached(), {'val1.val1': {None: ([], None)}}) cfg.option('val1.val1').value.set([undefined]) - compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}, - 'val1': {None: (set([]), None)}}) + compare(cfg._config.cfgimpl_get_settings()._p_.get_cached(), {None: {None: (set(['cache', 'disabled', 'frozen', 'hidden', 'validator', 'warnings']), None)}}) assert cfg._config.cfgimpl_get_values()._p_.get_cached() == {} cfg.config.dict() diff --git a/test/test_requires.py b/test/test_requires.py index 37c8872..cca6174 100644 --- a/test/test_requires.py +++ b/test/test_requires.py @@ -970,6 +970,62 @@ def test_master_slave_requires(): assert isinstance(ret['ip_admin_eth0.netmask_admin_eth0'][1], PropertiesOptionError) +def test_master_slave_requires_master(): + activate = BoolOption('activate', "Activer l'accès au réseau", True) + ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True, + requires=[{'option': activate, 'expected': False, 'action': 'disabled'}]) + netmask_admin_eth0 = StrOption('netmask_admin_eth0', "masque du sous-réseau", multi=True) + interface1 = MasterSlaves('ip_admin_eth0', '', [ip_admin_eth0, netmask_admin_eth0]) + maconfig = OptionDescription('toto', '', [activate, interface1]) + api = Config(maconfig) + api.property.read_write() + # + assert api.option('ip_admin_eth0.ip_admin_eth0').value.get() == [] + api.option('ip_admin_eth0.ip_admin_eth0').value.set(['192.168.1.2']) + assert api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get() is None + assert api.option('ip_admin_eth0.ip_admin_eth0').value.get() == ['192.168.1.2'] + # + api.option('activate').value.set(False) + raises(PropertiesOptionError, "api.option('ip_admin_eth0.ip_admin_eth0').value.get()") + raises(PropertiesOptionError, "api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get()") + # + api.option('activate').value.set(True) + assert api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get() is None + # + api.option('activate').value.set(False) + raises(PropertiesOptionError, "api.option('ip_admin_eth0.ip_admin_eth0').value.get()") + raises(PropertiesOptionError, "api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get()") + assert api.config.dict() == {'activate': False} + + +def test_master_slave_requires_masterslaves(): + activate = BoolOption('activate', "Activer l'accès au réseau", True) + ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True) + netmask_admin_eth0 = StrOption('netmask_admin_eth0', "masque du sous-réseau", multi=True) + interface1 = MasterSlaves('ip_admin_eth0', '', [ip_admin_eth0, netmask_admin_eth0], + requires=[{'option': activate, 'expected': False, 'action': 'disabled'}]) + maconfig = OptionDescription('toto', '', [activate, interface1]) + api = Config(maconfig) + api.property.read_write() + # + assert api.option('ip_admin_eth0.ip_admin_eth0').value.get() == [] + api.option('ip_admin_eth0.ip_admin_eth0').value.set(['192.168.1.2']) + assert api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get() is None + assert api.option('ip_admin_eth0.ip_admin_eth0').value.get() == ['192.168.1.2'] + # + api.option('activate').value.set(False) + raises(PropertiesOptionError, "api.option('ip_admin_eth0.ip_admin_eth0').value.get()") + raises(PropertiesOptionError, "api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get()") + # + api.option('activate').value.set(True) + assert api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get() is None + # + api.option('activate').value.set(False) + raises(PropertiesOptionError, "api.option('ip_admin_eth0.ip_admin_eth0').value.get()") + raises(PropertiesOptionError, "api.option('ip_admin_eth0.netmask_admin_eth0', 0).value.get()") + assert api.config.dict() == {'activate': False} + + def test_master_slave_requires_no_master(): activate = BoolOption('activate', "Activer l'accès au réseau", True) ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True) diff --git a/tiramisu/config.py b/tiramisu/config.py index 7f2124e..be01ac5 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -24,9 +24,7 @@ from copy import copy from .error import PropertiesOptionError, ConfigError, ConflictError, SlaveError -from .option.syndynoptiondescription import SynDynOptionDescription -from .option.dynoptiondescription import DynOptionDescription -from .option.masterslave import MasterSlaves +from .option import SynDynOptionDescription, DynOptionDescription, MasterSlaves from .option.baseoption import BaseOption, valid_name from .setting import OptionBag, ConfigBag, groups, Settings, undefined from .storage import get_storages, get_default_values_storages diff --git a/tiramisu/option/__init__.py b/tiramisu/option/__init__.py index e22f41f..a2c295c 100644 --- a/tiramisu/option/__init__.py +++ b/tiramisu/option/__init__.py @@ -1,7 +1,7 @@ from .optiondescription import OptionDescription from .dynoptiondescription import DynOptionDescription from .syndynoptiondescription import SynDynOptionDescription -from .masterslave import MasterSlaves +from .masterslaves import MasterSlaves from .baseoption import submulti from .symlinkoption import SymLinkOption from .dynsymlinkoption import DynSymLinkOption diff --git a/tiramisu/option/baseoption.py b/tiramisu/option/baseoption.py index e540cb5..06bcf57 100644 --- a/tiramisu/option/baseoption.py +++ b/tiramisu/option/baseoption.py @@ -98,11 +98,7 @@ class Base(object): raise TypeError(_('invalid properties type {0} for {1},' ' must be a frozenset').format(type(properties), name)) - if calc_properties != frozenset([]) and properties: - set_forbidden_properties = calc_properties & properties - if set_forbidden_properties != frozenset(): - raise ValueError(_('conflict: properties already set in ' - 'requirement {0}').format(display_list(list(set_forbidden_properties)))) + self.validate_properties(name, calc_properties, properties) _setattr = object.__setattr__ _setattr(self, '_name', name) _setattr(self, '_informations', {'doc': doc}) @@ -113,6 +109,13 @@ class Base(object): if properties: _setattr(self, '_properties', properties) + def validate_properties(self, name, calc_properties, properties): + 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)), + name)) + def _get_function_args(self, function): args = set() diff --git a/tiramisu/option/ipoption.py b/tiramisu/option/ipoption.py index 087e166..0299b5b 100644 --- a/tiramisu/option/ipoption.py +++ b/tiramisu/option/ipoption.py @@ -111,7 +111,7 @@ class IPOption(Option): raise ValueError(msg.format(network, netmask, opts[1].impl_get_display_name(), - opts[2].impl_getname(), + opts[2].impl_get_display_name(), ip)) # test if ip is not network/broadcast IP opts[2]._cons_ip_netmask(current_opt, diff --git a/tiramisu/option/masterslave.py b/tiramisu/option/masterslaves.py similarity index 84% rename from tiramisu/option/masterslave.py rename to tiramisu/option/masterslaves.py index a5bedbc..b0011cb 100644 --- a/tiramisu/option/masterslave.py +++ b/tiramisu/option/masterslaves.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -"master slave support" +"master slaves support" # Copyright (C) 2014-2018 Team tiramisu (see AUTHORS for all contributors) # # This program is free software: you can redistribute it and/or modify it @@ -27,7 +27,7 @@ from ..i18n import _ from ..setting import groups, undefined, OptionBag from .optiondescription import OptionDescription from .option import Option -from ..error import SlaveError, PropertiesOptionError +from ..error import SlaveError, PropertiesOptionError, RequirementError from ..function import ParamOption @@ -69,9 +69,9 @@ class MasterSlaves(OptionDescription): self.impl_get_display_name())) # no empty property for save if idx != 0: - properties = list(child._properties) - properties.remove('empty') - child._properties = frozenset(properties) + child_properties = list(child._properties) + child_properties.remove('empty') + child._properties = frozenset(child_properties) slaves.append(child) child._add_dependency(self) child._master_slaves = weakref.ref(self) @@ -82,6 +82,21 @@ class MasterSlaves(OptionDescription): if callbk.option in slaves: raise ValueError(_("callback of master's option shall " "not refered a slave's ones")) + # master should not have requires, only MasterSlaves should have + # so move requires to MasterSlaves + # if MasterSlaves has requires to, cannot manage the move so raises + master_requires = getattr(master, '_requires', None) + if master_requires: + if self.impl_getrequires(): + raise RequirementError(_('master {} in MasterSlaves {} cannot have both requirement' + '').format(master.impl_getname(), + self.impl_getname())) + master_calproperties = getattr(master, '_calc_properties', None) + if master_calproperties: + if properties is not None: + self.validate_properties(name, master_calproperties, frozenset(properties)) + setattr(self, '_calc_properties', master_calproperties) + setattr(self, '_requires', master_requires) def is_master(self, opt): master = self._children[0][0] @@ -158,18 +173,24 @@ class MasterSlaves(OptionDescription): resetted_opts): master = self.getmaster() slaves = self.getslaves() - self._reset_cache(master, + self._reset_cache(path, + master, slaves, values, settings, resetted_opts) def _reset_cache(self, + path, master, slaves, values, settings, resetted_opts): + super().reset_cache(path, + values, + settings, + resetted_opts) context = values._getcontext() mpath = master.impl_getpath(context) master.reset_cache(mpath, diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index f478cce..4828ed5 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -673,9 +673,9 @@ class Option(OnlyOption): log.debug(_('_cons_not_equal: {} are not different').format(display_list(equal))) if is_current: if warnings_only: - msg = _('should be different from the value of {}') + msg = _('should be different from the value of "{}"') else: - msg = _('must be different from the value of {}') + msg = _('must be different from the value of "{}"') else: if warnings_only: msg = _('value for {} should be different') diff --git a/tiramisu/option/syndynoptiondescription.py b/tiramisu/option/syndynoptiondescription.py index 004b2a4..f2fda71 100644 --- a/tiramisu/option/syndynoptiondescription.py +++ b/tiramisu/option/syndynoptiondescription.py @@ -103,7 +103,8 @@ class SynDynOptionDescription(object): if self.impl_get_group_type() == groups.master: master = self.getmaster() slaves = self.getslaves() - self._reset_cache(master, + self._reset_cache(path, + master, slaves, values, settings,