if we delete all reference to a Config and we have reference to old SubConfig, Values, Multi or Settings, make a ConfigError instead of AttributError on NoneType object

This commit is contained in:
Emmanuel Garette 2014-01-25 11:20:11 +01:00
parent 21a67971c5
commit 138018dfe9
4 changed files with 110 additions and 43 deletions

View 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()')

View 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:

View 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()

View 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