simplify _setvalue
authorEmmanuel Garette <egarette@cadoles.com>
Thu, 31 Dec 2015 17:20:36 +0000 (18:20 +0100)
committerEmmanuel Garette <egarette@cadoles.com>
Thu, 31 Dec 2015 17:20:36 +0000 (18:20 +0100)
test/test_freeze.py
tiramisu/config.py
tiramisu/option/masterslave.py
tiramisu/value.py

index 3d9a923..4e1ffaf 100644 (file)
@@ -51,6 +51,8 @@ def test_freeze_whole_config():
     except PropertiesOptionError as err:
         prop = err.proptype
     assert 'frozen' in prop
+    assert conf.gc.dummy is False
+    #
     setting.remove('everything_frozen')
     conf.gc.dummy = True
     assert conf.gc.dummy is True
index 9e20c59..f8564b2 100644 (file)
@@ -24,7 +24,7 @@ import weakref
 
 from .error import PropertiesOptionError, ConfigError, ConflictError
 from .option import OptionDescription, Option, SymLinkOption, \
-    DynSymLinkOption
+    DynSymLinkOption, SynDynOptionDescription
 from .option.baseoption import valid_name
 from .setting import groups, Settings, default_encoding, undefined
 from .storage import get_storages, get_storage, set_storage, \
@@ -52,10 +52,8 @@ class SubConfig(object):
         """
         # main option description
         error = False
-        try:
-            if descr is not None and not descr.impl_is_optiondescription():  # pragma: optional cover
-                error = True
-        except AttributeError:
+        if descr is not None and not isinstance(descr, OptionDescription) and \
+                                  not isinstance(descr, SynDynOptionDescription):  # pragma: optional cover
             error = True
         if error:
             raise TypeError(_('descr must be an optiondescription, not {0}'
@@ -309,7 +307,7 @@ class SubConfig(object):
                                                  force_permissive=force_permissive)
 
     def find_first(self, bytype=None, byname=None, byvalue=undefined,
-                   type_='option', display_error=True, check_properties=True,
+                   type_='option', raise_if_not_found=True, check_properties=True,
                    force_permissive=False):
         """
             finds an option recursively in the config
@@ -321,12 +319,12 @@ class SubConfig(object):
         """
         return self._cfgimpl_get_context()._find(
             bytype, byname, byvalue, first=True, type_=type_,
-            _subpath=self.cfgimpl_get_path(False), display_error=display_error,
+            _subpath=self.cfgimpl_get_path(False), raise_if_not_found=raise_if_not_found,
             check_properties=check_properties,
             force_permissive=force_permissive)
 
     def _find(self, bytype, byname, byvalue, first, type_='option',
-              _subpath=None, check_properties=True, display_error=True,
+              _subpath=None, check_properties=True, raise_if_not_found=True,
               force_permissive=False, only_path=undefined,
               only_option=undefined, setting_properties=undefined):
         """
@@ -385,16 +383,13 @@ class SubConfig(object):
                 return retval
             else:
                 find_results.append(retval)
-        return self._find_return_results(find_results, display_error)
+        return self._find_return_results(find_results, raise_if_not_found)
 
-    def _find_return_results(self, find_results, display_error):
+    def _find_return_results(self, find_results, raise_if_not_found):
         if find_results == []:  # pragma: optional cover
-            if display_error:
+            if raise_if_not_found:
                 raise AttributeError(_("no option found in config"
                                        " with these criteria"))
-            else:
-                # translation is slow
-                raise AttributeError()
         else:
             return find_results
 
@@ -749,7 +744,7 @@ class GroupConfig(_CommonConfig):
                 pass
 
     def find_firsts(self, byname=None, bypath=undefined, byoption=undefined,
-                    byvalue=undefined, display_error=True, _sub=False,
+                    byvalue=undefined, raise_if_not_found=True, _sub=False,
                     check_properties=True):
         """Find first not in current GroupConfig, but in each children
         """
@@ -762,32 +757,28 @@ class GroupConfig(_CommonConfig):
             bypath = self._find(bytype=None, byvalue=undefined, byname=byname,
                                 first=True, type_='path',
                                 check_properties=None,
-                                display_error=display_error)
+                                raise_if_not_found=raise_if_not_found)
             byname = None
             byoption = self.cfgimpl_get_description(
             ).impl_get_opt_by_path(bypath)
 
         for child in self._impl_children:
-            try:
-                if isinstance(child, GroupConfig):
-                    ret.extend(child.find_firsts(byname=byname, bypath=bypath,
-                                                 byoption=byoption,
-                                                 byvalue=byvalue,
-                                                 check_properties=check_properties,
-                                                 display_error=False,
-                                                 _sub=True))
-                else:
-                    child._find(None, byname, byvalue, first=True,
-                                type_='path', display_error=False,
-                                check_properties=check_properties,
-                                only_path=bypath, only_option=byoption)
-                    ret.append(child)
-            except AttributeError:
-                pass
+            if isinstance(child, GroupConfig):
+                ret.extend(child.find_firsts(byname=byname, bypath=bypath,
+                                             byoption=byoption,
+                                             byvalue=byvalue,
+                                             check_properties=check_properties,
+                                             raise_if_not_found=False,
+                                             _sub=True))
+            elif child._find(None, byname, byvalue, first=True,
+                             type_='path', raise_if_not_found=False,
+                             check_properties=check_properties,
+                             only_path=bypath, only_option=byoption):
+                ret.append(child)
         if _sub:
             return ret
         else:
-            return GroupConfig(self._find_return_results(ret, display_error))
+            return GroupConfig(self._find_return_results(ret, raise_if_not_found))
 
     def __repr__(self):
         return object.__repr__(self)
@@ -796,10 +787,7 @@ class GroupConfig(_CommonConfig):
         ret = ''
         for child in self._impl_children:
             ret += '({0})\n'.format(child._impl_name)
-        try:
-            ret += super(GroupConfig, self).__str__()
-        except ConfigError:
-            pass
+        ret += super(GroupConfig, self).__str__()
         return ret
 
     def getattr(self, name, force_permissive=False, validate=True,
index a0509c4..b982aba 100644 (file)
@@ -218,7 +218,7 @@ class MasterSlaves(object):
                 raise err
         return multi
 
-    def setitem(self, values, opt, value, path):
+    def validate(self, values, opt, value, path):
         if self.is_master(opt):
             masterlen = len(value)
             #for regen slave path
index d5f53c0..bb75468 100644 (file)
@@ -372,50 +372,45 @@ class Values(object):
         setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=False)
         if 'validator' in setting_properties:
             fake_context = context._gen_fake_values()
-            fake_context.cfgimpl_get_values()._setitem(opt, value, path,
-                                                       force_permissive,
-                                                       check_frozen,
-                                                       setting_properties)
+            fake_values = fake_context.cfgimpl_get_values()
+            fake_values.validate(opt, value, path, check_frozen,
+                                 force_permissive, setting_properties)
+            fake_values._setvalue(opt, path, value)
             opt.impl_validate(value, fake_context)
-        self._setitem(opt, value, path, force_permissive, check_frozen,
-                      setting_properties, validate_properties=False)
-
-    def _setitem(self, opt, value, path, force_permissive, check_frozen,
-                 setting_properties, validate_properties=True):
-        if opt.impl_is_master_slaves():
-            opt.impl_get_master_slaves().setitem(self, opt, value, path)
-        self._setvalue(opt, path, value, force_permissive=force_permissive,
-                       check_frozen=check_frozen,
-                       setting_properties=setting_properties,
-                       validate_properties=validate_properties)
-
-    def _setvalue(self, opt, path, value, force_permissive=False,
-                  check_frozen=True, validate_properties=True,
-                  setting_properties=undefined, index=None):
+        self._setvalue(opt, path, value)
+
+    def _setvalue(self, opt, path, value):
         context = self._getcontext()
         context.cfgimpl_reset_cache()
+        owner = context.cfgimpl_get_settings().getowner()
+        # in storage, value must not be a multi
         if isinstance(value, Multi):
             value = list(value)
             if opt.impl_is_submulti():
                 for idx, val in enumerate(value):
                     if isinstance(val, SubMulti):
                         value[idx] = list(val)
-        setting = context.cfgimpl_get_settings()
-        owner = setting.getowner()
         #FIXME pourquoi là et pas dans masterslaves ??
-        if opt.impl_is_master_slaves('slave') and index is None:
+        if opt.impl_is_master_slaves('slave'):
             self._p_.resetvalue(path)
             for idx, val in enumerate(value):
                 self._p_.setvalue(path, val, owner, idx)
         else:
-            self._p_.setvalue(path, value, owner, index)
-        if validate_properties:
-            props = setting.validate_properties(opt, False, check_frozen,
-                                                value=value, path=path,
-                                                force_permissive=force_permissive,
-                                                setting_properties=setting_properties)
-            if props:
-                raise props
+            self._p_.setvalue(path, value, owner, None)
+
+    def validate(self, opt, value, path, check_frozen=True, force_permissive=False,
+                 setting_properties=undefined, valid_masterslave=True):
+        if valid_masterslave and opt.impl_is_master_slaves():
+            opt.impl_get_master_slaves().validate(self, opt, value, path)
+        props = self._getcontext().cfgimpl_get_settings().validate_properties(opt,
+                                                                              False,
+                                                                              check_frozen,
+                                                                              value=value,
+                                                                              path=path,
+                                                                              force_permissive=force_permissive,
+                                                                              setting_properties=setting_properties)
+        if props:
+            raise props
 
     def _is_meta(self, opt, path):
         context = self._getcontext()
@@ -837,9 +832,11 @@ class Multi(list):
         return ret
 
     def _store(self, force=False):
-        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path,
-                                                          self,
-                                                          validate_properties=not force)
+        values = self._getcontext().cfgimpl_get_values()
+        if not force:
+            #FIXME could get properties an pass it
+            values.validate(self.opt, self, self.path, valid_masterslave=False)
+        values._setvalue(self.opt, self.path, self)
 
 
 class SubMulti(Multi):
@@ -864,9 +861,9 @@ class SubMulti(Multi):
 
     def _store(self, force=False):
         #force is unused here
-        self._getcontext().cfgimpl_get_values()._setvalue(self.opt,
-                                                          self.path,
-                                                          self.submulti())
+        values = self._getcontext().cfgimpl_get_values()
+        values.validate(self.opt, self, self.path, valid_masterslave=False)
+        values._setvalue(self.opt, self.path, self.submulti())
 
     def _validate(self, value, fake_context, force_index, submulti=False):
         if value is not None: