if we delete all reference to a Config and we have reference to old SubConfig, Values...
authorEmmanuel Garette <egarette@cadoles.com>
Sat, 25 Jan 2014 10:20:11 +0000 (11:20 +0100)
committerEmmanuel Garette <egarette@cadoles.com>
Sat, 25 Jan 2014 10:20:11 +0000 (11:20 +0100)
test/test_config.py
tiramisu/config.py
tiramisu/setting.py
tiramisu/value.py

index 5cdbe7d..6db3fcf 100644 (file)
@@ -9,7 +9,7 @@ from py.test import raises
 from tiramisu.config import Config
 from tiramisu.option import IntOption, FloatOption, StrOption, ChoiceOption, \
     BoolOption, UnicodeOption, OptionDescription
-from tiramisu.error import ConflictError
+from tiramisu.error import ConflictError, ConfigError
 
 
 def make_description():
@@ -273,4 +273,24 @@ def test_no_validation():
     setting.append('validator')
     raises(ValueError, 'c.test1')
     del(c.test1)
-    assert c.test1 == None
+    assert c.test1 is None
+
+
+def test_delete_config_with_subconfig():
+    test = IntOption('test', '')
+    multi = IntOption('multi', '', multi=True)
+    od = OptionDescription('od', '', [test, multi])
+    odroot = OptionDescription('odroot', '', [od])
+    c = Config(odroot)
+    sub = c.od
+    val = c.cfgimpl_get_values()
+    setting = c.cfgimpl_get_settings()
+    val[test]
+    val[multi]
+    setting[test]
+    sub.make_dict()
+    del(c)
+    raises(ConfigError, 'val[test]')
+    raises(ConfigError, 'val[multi]')
+    raises(ConfigError, 'setting[test]')
+    raises(ConfigError, 'sub.make_dict()')
index 1381d57..4bc0e15 100644 (file)
@@ -156,7 +156,15 @@ class SubConfig(object):
     __repr__ = __str__
 
     def _cfgimpl_get_context(self):
-        return self._impl_context()
+        """context could be None, we need to test it
+        context is None only if all reference to `Config` object is deleted
+        (for example we delete a `Config` and we manipulate a reference to
+        old `SubConfig`, `Values`, `Multi` or `Settings`)
+        """
+        context = self._impl_context()
+        if context is None:
+            raise ConfigError(_('the context does not exist anymore'))
+        return context
 
     def cfgimpl_get_description(self):
         if self._impl_descr is None:
index 02da5ae..101b551 100644 (file)
@@ -24,7 +24,7 @@ from time import time
 from copy import copy
 import weakref
 from tiramisu.error import (RequirementError, PropertiesOptionError,
-                            ConstError)
+                            ConstError, ConfigError)
 from tiramisu.i18n import _
 
 
@@ -328,6 +328,17 @@ class Settings(object):
         self.context = weakref.ref(context)
         self._p_ = storage
 
+    def _getcontext(self):
+        """context could be None, we need to test it
+        context is None only if all reference to `Config` object is deleted
+        (for example we delete a `Config` and we manipulate a reference to
+        old `SubConfig`, `Values`, `Multi` or `Settings`)
+        """
+        context = self.context()
+        if context is None:
+            raise ConfigError(_('the context does not exist anymore'))
+        return context
+
     #____________________________________________________________
     # properties methods
     def __contains__(self, propname):
@@ -357,7 +368,7 @@ class Settings(object):
             if opt is not None and _path is None:
                 _path = self._get_path_by_opt(opt)
             self._p_.reset_properties(_path)
-        self.context().cfgimpl_reset_cache()
+        self._getcontext().cfgimpl_reset_cache()
 
     def _getproperties(self, opt=None, path=None, is_apply_req=True):
         if opt is None:
@@ -410,7 +421,7 @@ class Settings(object):
                 self._p_.reset_properties(path)
             else:
                 self._p_.setproperties(path, properties)
-        self.context().cfgimpl_reset_cache()
+        self._getcontext().cfgimpl_reset_cache()
 
     #____________________________________________________________
     def validate_properties(self, opt_or_descr, is_descr, is_write, path,
@@ -458,7 +469,7 @@ class Settings(object):
             properties -= frozenset(('mandatory', 'frozen'))
         else:
             if 'mandatory' in properties and \
-                    not self.context().cfgimpl_get_values()._isempty(
+                    not self._getcontext().cfgimpl_get_values()._isempty(
                         opt_or_descr, value):
                 properties.remove('mandatory')
             if is_write and 'everything_frozen' in self_properties:
@@ -581,6 +592,7 @@ class Settings(object):
 
         # filters the callbacks
         calc_properties = set()
+        context = self._getcontext()
         for requires in opt._requires:
             for require in requires:
                 option, expected, action, inverse, \
@@ -592,8 +604,7 @@ class Settings(object):
                                              " '{0}' with requirement on: "
                                              "'{1}'").format(path, reqpath))
                 try:
-                    value = self.context()._getattr(reqpath,
-                                                    force_permissive=True)
+                    value = context._getattr(reqpath, force_permissive=True)
                 except PropertiesOptionError as err:
                     if not transitive:
                         continue
@@ -622,7 +633,7 @@ class Settings(object):
         :param opt: `Option`'s object
         :returns: path
         """
-        return self.context().cfgimpl_get_description().impl_get_path_by_opt(opt)
+        return self._getcontext().cfgimpl_get_description().impl_get_path_by_opt(opt)
 
     def get_modified_properties(self):
         return self._p_.get_modified_properties()
index 8c50373..a02196a 100644 (file)
@@ -46,13 +46,24 @@ class Values(object):
         # the storage type is dictionary or sqlite3
         self._p_ = storage
 
+    def _getcontext(self):
+        """context could be None, we need to test it
+        context is None only if all reference to `Config` object is deleted
+        (for example we delete a `Config` and we manipulate a reference to
+        old `SubConfig`, `Values`, `Multi` or `Settings`)
+        """
+        context = self.context()
+        if context is None:
+            raise ConfigError(_('the context does not exist anymore'))
+        return context
+
     def _getdefault(self, opt):
         """
         actually retrieves the default value
 
         :param opt: the `option.Option()` object
         """
-        meta = self.context().cfgimpl_get_meta()
+        meta = self._getcontext().cfgimpl_get_meta()
         if meta is not None:
             value = meta.cfgimpl_get_values()[opt]
         else:
@@ -105,11 +116,11 @@ class Values(object):
         if path is None:
             path = self._get_opt_path(opt)
         if self._p_.hasvalue(path):
-            setting = self.context().cfgimpl_get_settings()
+            context = self._getcontext()
+            setting = context.cfgimpl_get_settings()
             opt.impl_validate(opt.impl_getdefault(),
-                              self.context(),
-                              'validator' in setting)
-            self.context().cfgimpl_reset_cache()
+                              context, 'validator' in setting)
+            context.cfgimpl_reset_cache()
             if (opt.impl_is_multi() and
                     opt.impl_get_multitype() == multitypes.master):
                 for slave in opt.impl_get_master_slaves():
@@ -137,7 +148,7 @@ class Values(object):
         callback, callback_params = opt._callback
         if callback_params is None:
             callback_params = {}
-        return carry_out_calculation(opt, config=self.context(),
+        return carry_out_calculation(opt, config=self._getcontext(),
                                      callback=callback,
                                      callback_params=callback_params,
                                      index=index, max_len=max_len)
@@ -151,7 +162,7 @@ class Values(object):
         if path is None:
             path = self._get_opt_path(opt)
         ntime = None
-        setting = self.context().cfgimpl_get_settings()
+        setting = self._getcontext().cfgimpl_get_settings()
         if 'cache' in setting and self._p_.hascache(path):
             if 'expire' in setting:
                 ntime = int(time())
@@ -176,7 +187,8 @@ class Values(object):
     def _getitem(self, opt, path, validate, force_permissive, force_properties,
                  validate_properties):
         # options with callbacks
-        setting = self.context().cfgimpl_get_settings()
+        context = self._getcontext()
+        setting = context.cfgimpl_get_settings()
         is_frozen = 'frozen' in setting[opt]
         # For calculating properties, we need value (ie for mandatory value).
         # If value is calculating with a PropertiesOptionError's option
@@ -196,7 +208,7 @@ class Values(object):
             if (opt.impl_is_multi() and
                     opt.impl_get_multitype() == multitypes.slave):
                 masterp = self._get_opt_path(opt.impl_get_master_slaves())
-                mastervalue = getattr(self.context(), masterp)
+                mastervalue = getattr(context, masterp)
                 lenmaster = len(mastervalue)
                 if lenmaster == 0:
                     value = []
@@ -230,7 +242,7 @@ class Values(object):
         else:
             value = self._getvalue(opt, path, validate)
         if config_error is None and validate:
-            opt.impl_validate(value, self.context(), 'validator' in setting)
+            opt.impl_validate(value, context, 'validator' in setting)
         if config_error is None and self._is_default_owner(path) and \
                 'force_store_value' in setting[opt]:
             self.setitem(opt, value, path, is_write=False)
@@ -251,8 +263,9 @@ class Values(object):
         # is_write is, for example, used with "force_store_value"
         # user didn't change value, so not write
         # valid opt
-        opt.impl_validate(value, self.context(),
-                          'validator' in self.context().cfgimpl_get_settings())
+        context = self._getcontext()
+        opt.impl_validate(value, context,
+                          'validator' in context.cfgimpl_get_settings())
         if opt.impl_is_multi() and not isinstance(value, Multi):
             value = Multi(value, self.context, opt, path, setitem=True)
         self._setvalue(opt, path, value, force_permissive=force_permissive,
@@ -261,14 +274,15 @@ class Values(object):
     def _setvalue(self, opt, path, value, force_permissive=False,
                   force_properties=None,
                   is_write=True, validate_properties=True):
-        self.context().cfgimpl_reset_cache()
+        context = self._getcontext()
+        context.cfgimpl_reset_cache()
         if validate_properties:
-            setting = self.context().cfgimpl_get_settings()
+            setting = context.cfgimpl_get_settings()
             setting.validate_properties(opt, False, is_write,
                                         value=value, path=path,
                                         force_permissive=force_permissive,
                                         force_properties=force_properties)
-        owner = self.context().cfgimpl_get_settings().getowner()
+        owner = context.cfgimpl_get_settings().getowner()
         self._p_.setvalue(path, value, owner)
 
     def getowner(self, opt):
@@ -285,7 +299,7 @@ class Values(object):
 
     def _getowner(self, path):
         owner = self._p_.getowner(path, owners.default)
-        meta = self.context().cfgimpl_get_meta()
+        meta = self._getcontext().cfgimpl_get_meta()
         if owner is owners.default and meta is not None:
             owner = meta.cfgimpl_get_values()._getowner(path)
         return owner
@@ -337,7 +351,7 @@ class Values(object):
         :param opt: the `option.Option` object
         :returns: a string with points like "gc.dummy.my_option"
         """
-        return self.context().cfgimpl_get_description().impl_get_path_by_opt(opt)
+        return self._getcontext().cfgimpl_get_description().impl_get_path_by_opt(opt)
 
     # information
     def set_information(self, key, value):
@@ -402,12 +416,24 @@ class Multi(list):
             self._valid_master(value)
         super(Multi, self).__init__(value)
 
+    def _getcontext(self):
+        """context could be None, we need to test it
+        context is None only if all reference to `Config` object is deleted
+        (for example we delete a `Config` and we manipulate a reference to
+        old `SubConfig`, `Values`, `Multi` or `Settings`)
+        """
+        context = self.context()
+        if context is None:
+            raise ConfigError(_('the context does not exist anymore'))
+        return context
+
     def _valid_slave(self, value, setitem):
         #if slave, had values until master's one
-        values = self.context().cfgimpl_get_values()
-        masterp = self.context().cfgimpl_get_description().impl_get_path_by_opt(
+        context = self._getcontext()
+        values = context.cfgimpl_get_values()
+        masterp = context.cfgimpl_get_description().impl_get_path_by_opt(
             self.opt.impl_get_master_slaves())
-        mastervalue = getattr(self.context(), masterp)
+        mastervalue = getattr(context, masterp)
         masterlen = len(mastervalue)
         valuelen = len(value)
         is_default_owner = not values._is_default_owner(self.path) or setitem
@@ -431,7 +457,7 @@ class Multi(list):
 
     def _valid_master(self, value):
         masterlen = len(value)
-        values = self.context().cfgimpl_get_values()
+        values = self._getcontext().cfgimpl_get_values()
         for slave in self.opt._master_slaves:
             path = values._get_opt_path(slave)
             if not values._is_default_owner(path):
@@ -458,18 +484,19 @@ class Multi(list):
         self._validate(value, index)
         #assume not checking mandatory property
         super(Multi, self).__setitem__(index, value)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
+        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
 
     def append(self, value=undefined, force=False):
         """the list value can be updated (appened)
         only if the option is a master
         """
+        context = self._getcontext()
         if not force:
             if self.opt.impl_get_multitype() == multitypes.slave:
                 raise SlaveError(_("cannot append a value on a multi option {0}"
                                    " which is a slave").format(self.opt._name))
             elif self.opt.impl_get_multitype() == multitypes.master:
-                values = self.context().cfgimpl_get_values()
+                values = context.cfgimpl_get_values()
                 if value is undefined and self.opt.impl_has_callback():
                     value = values._getcallback_value(self.opt)
                     #Force None il return a list
@@ -480,9 +507,9 @@ class Multi(list):
             value = self.opt.impl_getdefault_multi()
         self._validate(value, index)
         super(Multi, self).append(value)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path,
-                                                      self,
-                                                      validate_properties=not force)
+        context.cfgimpl_get_values()._setvalue(self.opt, self.path,
+                                               self,
+                                               validate_properties=not force)
         if not force and self.opt.impl_get_multitype() == multitypes.master:
             for slave in self.opt.impl_get_master_slaves():
                 path = values._get_opt_path(slave)
@@ -513,7 +540,7 @@ class Multi(list):
             super(Multi, self).sort(key=key, reverse=reverse)
         else:
             super(Multi, self).sort(cmp=cmp, key=key, reverse=reverse)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
+        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
 
     def reverse(self):
         if self.opt.impl_get_multitype() in [multitypes.slave,
@@ -521,7 +548,7 @@ class Multi(list):
             raise SlaveError(_("cannot reverse multi option {0} if master or "
                                "slave").format(self.opt._name))
         super(Multi, self).reverse()
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
+        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
 
     def insert(self, index, obj):
         if self.opt.impl_get_multitype() in [multitypes.slave,
@@ -529,7 +556,7 @@ class Multi(list):
             raise SlaveError(_("cannot insert multi option {0} if master or "
                                "slave").format(self.opt._name))
         super(Multi, self).insert(index, obj)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
+        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
 
     def extend(self, iterable):
         if self.opt.impl_get_multitype() in [multitypes.slave,
@@ -537,12 +564,12 @@ class Multi(list):
             raise SlaveError(_("cannot extend multi option {0} if master or "
                                "slave").format(self.opt._name))
         super(Multi, self).extend(iterable)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
+        self._getcontext().cfgimpl_get_values()._setvalue(self.opt, self.path, self)
 
     def _validate(self, value, force_index):
         if value is not None:
             try:
-                self.opt.impl_validate(value, context=self.context(),
+                self.opt.impl_validate(value, context=self._getcontext(),
                                        force_index=force_index)
             except ValueError as err:
                 raise ValueError(_("invalid value {0} "
@@ -560,13 +587,14 @@ class Multi(list):
         :type force: boolean
         :returns: item at index
         """
+        context = self._getcontext()
         if not force:
             if self.opt.impl_get_multitype() == multitypes.slave:
                 raise SlaveError(_("cannot pop a value on a multi option {0}"
                                    " which is a slave").format(self.opt._name))
             elif self.opt.impl_get_multitype() == multitypes.master:
                 for slave in self.opt.impl_get_master_slaves():
-                    values = self.context().cfgimpl_get_values()
+                    values = context.cfgimpl_get_values()
                     if not values.is_default_owner(slave):
                         #get multi without valid properties
                         values.getitem(slave,
@@ -574,5 +602,5 @@ class Multi(list):
                                        ).pop(index, force=True)
         #set value without valid properties
         ret = super(Multi, self).pop(index)
-        self.context().cfgimpl_get_values()._setvalue(self.opt, self.path, self, validate_properties=not force)
+        context.cfgimpl_get_values()._setvalue(self.opt, self.path, self, validate_properties=not force)
         return ret