From 962b4eb66043316d09426ce9304f461be5a997f9 Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Tue, 11 Jul 2017 22:31:58 +0200 Subject: [PATCH] many improvment --- test/test_storage.py | 19 ++++++++++ tiramisu/config.py | 53 ++++++++++++++++++---------- tiramisu/option/optiondescription.py | 17 ++++----- tiramisu/storage/dictionary/value.py | 27 +++++++------- tiramisu/storage/sqlite3/setting.py | 13 ++----- tiramisu/storage/sqlite3/storage.py | 41 ++++++++++++++------- tiramisu/storage/sqlite3/value.py | 49 ++++++++++++------------- tiramisu/value.py | 41 +++++++++++---------- 8 files changed, 156 insertions(+), 104 deletions(-) diff --git a/test/test_storage.py b/test/test_storage.py index f1aee89..8e0771f 100644 --- a/test/test_storage.py +++ b/test/test_storage.py @@ -271,3 +271,22 @@ def test_two_different_persistents(): delete_session('config', 'test_persistent') delete_session('config', 'test_persistent2') + + +def test_two_different_information(): + b = BoolOption('b', '') + o = OptionDescription('od', '', [b]) + try: + c = Config(o, session_id='test_persistent', persistent=True) + c.impl_set_information('a', 'a') + d = Config(o, session_id='test_persistent2', persistent=True) + d.impl_set_information('a', 'b') + except ValueError: + # storage is not persistent + pass + else: + assert c.impl_get_information('a') == 'a' + assert d.impl_get_information('a') == 'b' + + delete_session('config', 'test_persistent') + delete_session('config', 'test_persistent2') diff --git a/tiramisu/config.py b/tiramisu/config.py index 5a63346..ce96f39 100644 --- a/tiramisu/config.py +++ b/tiramisu/config.py @@ -145,13 +145,14 @@ class SubConfig(object): settings._p_.reset_all_cache() def cfgimpl_get_home_by_path(self, path, force_permissive=False, - returns_raise=False): + returns_raise=False, _setting_properties=undefined): """:returns: tuple (config, name)""" path = path.split('.') for step in path[:-1]: self = self.getattr(step, force_permissive=force_permissive, - returns_raise=returns_raise) + returns_raise=returns_raise, + _setting_properties=_setting_properties) if isinstance(self, Exception): return self, None return self, path[-1] @@ -274,15 +275,20 @@ class SubConfig(object): self.setattr(name, value, force_permissive=force_permissive, not_raises=not_raises) - def setattr(self, name, value, force_permissive=False, not_raises=False, index=None): + def setattr(self, name, value, force_permissive=False, not_raises=False, index=None, + _setting_properties=undefined): if name.startswith('_impl_'): return object.__setattr__(self, name, value) + context = self._cfgimpl_get_context() + if _setting_properties is undefined: + _setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=True) if '.' in name: # pragma: optional cover homeconfig, name = self.cfgimpl_get_home_by_path(name, - force_permissive=force_permissive) + force_permissive=force_permissive, + _setting_properties=_setting_properties) return homeconfig.setattr(name, value, force_permissive, - not_raises, index=index) - context = self._cfgimpl_get_context() + not_raises, index=index, + _setting_properties=_setting_properties) child = self.cfgimpl_get_description().__getattr__(name, context=context) if isinstance(child, OptionDescription) or isinstance(child, SynDynOptionDescription): @@ -291,12 +297,14 @@ class SubConfig(object): not isinstance(child, DynSymLinkOption): # pragma: no dynoptiondescription cover path = context.cfgimpl_get_description().impl_get_path_by_opt( child._impl_getopt()) - context.setattr(path, value, force_permissive, not_raises, index=index) + context.setattr(path, value, force_permissive, not_raises, index=index, + _setting_properties=_setting_properties) else: subpath = self._get_subpath(name) self.cfgimpl_get_values().setitem(child, value, subpath, force_permissive=force_permissive, - not_raises=not_raises, index=index) + not_raises=not_raises, index=index, + _setting_properties=_setting_properties) def __delattr__(self, name): context = self._cfgimpl_get_context() @@ -329,10 +337,13 @@ class SubConfig(object): """ # attribute access by passing a path, # for instance getattr(self, "creole.general.family.adresse_ip_eth0") + context = self._cfgimpl_get_context() + if _setting_properties is undefined: + _setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=True) if '.' in name: homeconfig, name = self.cfgimpl_get_home_by_path( name, force_permissive=force_permissive, - returns_raise=returns_raise) + returns_raise=returns_raise, _setting_properties=_setting_properties) if isinstance(homeconfig, Exception): cfg = homeconfig else: @@ -342,12 +353,9 @@ class SubConfig(object): _self_properties=_self_properties, index=index, returns_raise=returns_raise) else: - context = self._cfgimpl_get_context() option = self.cfgimpl_get_description().__getattr__(name, context=context) subpath = self._get_subpath(name) - if _setting_properties is undefined: - _setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=True) if isinstance(option, DynSymLinkOption): cfg = self.cfgimpl_get_values()._get_cached_value( option, path=subpath, @@ -626,12 +634,12 @@ class _CommonConfig(SubConfig): "abstract base class for the Config, GroupConfig and the MetaConfig" __slots__ = ('_impl_values', '_impl_settings', '_impl_meta', '_impl_test') - def _impl_build_all_caches(self): + def _impl_build_all_caches(self, force_store_values): descr = self.cfgimpl_get_description() if not descr.impl_already_build_caches(): descr.impl_build_cache_option() descr.impl_build_cache(self) - descr.impl_build_force_store_values(self) + descr.impl_build_force_store_values(self, force_store_values) def read_only(self): "read only is a global config's setting, see `settings.py`" @@ -756,7 +764,7 @@ class Config(_CommonConfig): def __init__(self, descr, session_id=None, persistent=False, name=undefined, force_values=None, force_settings=None, - _duplicate=False, mandatory_name=False): + _duplicate=False, mandatory_name=False, _force_store_values=True): """ Configuration option management master class :param descr: describes the configuration schema @@ -789,7 +797,7 @@ class Config(_CommonConfig): #undocumented option used only in test script self._impl_test = False if _duplicate is False and (force_settings is None or force_values is None): - self._impl_build_all_caches() + self._impl_build_all_caches(_force_store_values) self._impl_name = name def impl_getname(self): @@ -921,8 +929,15 @@ class MetaConfig(GroupConfig): __slots__ = tuple() def __init__(self, children, session_id=None, persistent=False, - name=undefined): + name=undefined, optiondescription=None): descr = None + if optiondescription is not None: + new_children = [] + for child_session_id in children: + #FIXME _force_store_values doit etre a true si inexistant ! + new_children.append(Config(optiondescription, persistent=True, + session_id=child_session_id, _force_store_values=False)) + children = new_children for child in children: if not isinstance(child, _CommonConfig): raise TypeError(_("metaconfig's children " @@ -975,6 +990,6 @@ class MetaConfig(GroupConfig): setattr(self, path, value) - def new_config(self, session_id=None, name=undefined): + def new_config(self, session_id=None, persistent=False, name=undefined): return Config(self._impl_descr, _duplicate=True, session_id=session_id, name=name, - mandatory_name=True) + persistent=persistent, mandatory_name=True) diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index 9aa803a..c0b7382 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -202,14 +202,9 @@ class OptionDescription(BaseOption, StorageOptionDescription): self._set_readonly(False) - def impl_build_force_store_values(self, config): + def impl_build_force_store_values(self, config, force_store_values): session = config._impl_values._p_.getsession() for subpath, option in self._cache_force_store_values: - value = config.cfgimpl_get_values()._get_cached_value(option, - path=subpath, - validate=False, - trusted_cached_properties=False, - validate_properties=True) if option.impl_is_master_slaves('slave'): # problem with index raise ConfigError(_('a slave ({0}) cannot have ' @@ -217,8 +212,14 @@ class OptionDescription(BaseOption, StorageOptionDescription): if option._is_subdyn(): raise ConfigError(_('a dynoption ({0}) cannot have ' 'force_store_value property').format(subpath)) - config._impl_values._p_.setvalue(subpath, value, - owners.forced, None, session) + if force_store_values and not config._impl_values._p_.hasvalue(subpath, session): + value = config.cfgimpl_get_values()._get_cached_value(option, + path=subpath, + validate=False, + trusted_cached_properties=False, + validate_properties=True) + config._impl_values._p_.setvalue(subpath, value, + owners.forced, None, session) # ____________________________________________________________ def impl_set_group_type(self, group_type): diff --git a/tiramisu/storage/dictionary/value.py b/tiramisu/storage/dictionary/value.py index a37388a..94e11b4 100644 --- a/tiramisu/storage/dictionary/value.py +++ b/tiramisu/storage/dictionary/value.py @@ -152,26 +152,29 @@ class Values(Cache): return 0 return max(self._values[1][idx]) + 1 - def getowner(self, path, default, session, index=None, only_default=False): + def getowner(self, path, default, session, index=None, only_default=False, + with_value=False): """get owner for a path return: owner object """ if index is None: if only_default: if path in self._values[0]: - return undefined + owner = undefined else: - return default - val = self._getvalue(path, 3, index) - if val is undefined: - return default - return val - else: - value = self._getvalue(path, 3, index) - if value is undefined: - return default + owner = default else: - return value + owner = self._getvalue(path, 3, index) + if owner is undefined: + owner = default + else: + owner = self._getvalue(path, 3, index) + if owner is undefined: + owner = default + if with_value: + return owner, self._getvalue(path, 2, index) + else: + return owner def _getvalue(self, path, nb, index): """ diff --git a/tiramisu/storage/sqlite3/setting.py b/tiramisu/storage/sqlite3/setting.py index 8f4484a..6aab6dc 100644 --- a/tiramisu/storage/sqlite3/setting.py +++ b/tiramisu/storage/sqlite3/setting.py @@ -22,14 +22,7 @@ class Settings(Sqlite3DB): __slots__ = tuple() def __init__(self, storage): - settings_table = 'CREATE TABLE IF NOT EXISTS property(path TEXT,' - settings_table += 'properties text, session_id TEXT, PRIMARY KEY(path, session_id))' - permissives_table = 'CREATE TABLE IF NOT EXISTS permissive(path TEXT,' - permissives_table += 'permissives TEXT, session_id TEXT, PRIMARY KEY(path, session_id))' - # should init cache too super(Settings, self).__init__(storage) - self._storage.execute(settings_table, commit=False) - self._storage.execute(permissives_table) # properties def setproperties(self, path, properties): @@ -50,7 +43,7 @@ class Settings(Sqlite3DB): def getproperties(self, path, default_properties): path = self._sqlite_encode_path(path) value = self._storage.select("SELECT properties FROM property WHERE " - "path = ? AND session_id = ?", (path, self._session_id)) + "path = ? AND session_id = ? LIMIT 1", (path, self._session_id)) if value is None: return set(default_properties) else: @@ -59,7 +52,7 @@ class Settings(Sqlite3DB): def hasproperties(self, path): path = self._sqlite_encode_path(path) return self._storage.select("SELECT properties FROM property WHERE " - "path = ? AND session_id = ?", (path, self._session_id) + "path = ? AND session_id = ? LIMIT 1", (path, self._session_id) ) is not None def reset_all_properties(self): @@ -83,7 +76,7 @@ class Settings(Sqlite3DB): def getpermissive(self, path='_none'): permissives = self._storage.select("SELECT permissives FROM " - "permissive WHERE path = ? AND session_id = ?", + "permissive WHERE path = ? AND session_id = ? LIMIT 1", (path, self._session_id)) if permissives is None: return frozenset() diff --git a/tiramisu/storage/sqlite3/storage.py b/tiramisu/storage/sqlite3/storage.py index 7802dfb..2a4042c 100644 --- a/tiramisu/storage/sqlite3/storage.py +++ b/tiramisu/storage/sqlite3/storage.py @@ -40,18 +40,13 @@ def _gen_filename(): def list_sessions(): - conn = sqlite3.connect(_gen_filename()) - conn.text_factory = str - cursor = conn.cursor() + cursor = CONN.cursor() names = [row[0] for row in cursor.execute("SELECT DISTINCT session_id FROM value").fetchall()] - conn.close() return names def delete_session(session_id, session): - conn = sqlite3.connect(_gen_filename()) - conn.text_factory = str - cursor = conn.cursor() + cursor = CONN.cursor() cursor.execute("DELETE FROM property WHERE session_id = ?", (session_id,)) cursor.execute("DELETE FROM permissive WHERE session_id = ?", @@ -60,9 +55,10 @@ def delete_session(session_id, session): (session_id,)) cursor.execute("DELETE FROM information WHERE session_id = ?", (session_id,)) - conn.commit() - conn.close() + CONN.commit() +global CONN +CONN = None class Storage(object): __slots__ = ('_conn', '_cursor', 'persistent', 'session_id', 'serializable') @@ -72,9 +68,29 @@ class Storage(object): self.persistent = persistent self.serializable = self.persistent self.session_id = session_id - self._conn = sqlite3.connect(_gen_filename()) - self._conn.text_factory = str + global CONN + init = False + if CONN is None: + init = True + CONN = sqlite3.connect(_gen_filename()) + CONN.text_factory = str + self._conn = CONN self._cursor = self._conn.cursor() + if init: + settings_table = 'CREATE TABLE IF NOT EXISTS property(path TEXT,' + settings_table += 'properties text, session_id TEXT, PRIMARY KEY(path, session_id))' + permissives_table = 'CREATE TABLE IF NOT EXISTS permissive(path TEXT,' + permissives_table += 'permissives TEXT, session_id TEXT, PRIMARY KEY(path, session_id))' + values_table = 'CREATE TABLE IF NOT EXISTS value(path TEXT, ' + values_table += 'value TEXT, owner TEXT, idx INTEGER, session_id TEXT NOT NULL, '\ + 'PRIMARY KEY (path, idx, session_id))' + informations_table = 'CREATE TABLE IF NOT EXISTS information(key TEXT ' + informations_table += 'KEY, value TEXT, session_id TEXT NOT NULL, ' + informations_table += 'PRIMARY KEY (key, session_id))' + self.execute(values_table, commit=False) + self.execute(informations_table, commit=False) + self.execute(settings_table, commit=False) + self.execute(permissives_table) def execute(self, sql, params=None, commit=True): if params is None: @@ -92,7 +108,8 @@ class Storage(object): def __del__(self): self._cursor.close() - self._conn.close() + #FIXME + #self._conn.close() if not self.persistent: session = None if delete_session is not None: diff --git a/tiramisu/storage/sqlite3/value.py b/tiramisu/storage/sqlite3/value.py index b06bf9e..eb1b8b4 100644 --- a/tiramisu/storage/sqlite3/value.py +++ b/tiramisu/storage/sqlite3/value.py @@ -28,26 +28,19 @@ class Values(Sqlite3DB): def __init__(self, storage): """init plugin means create values storage """ - # should init cache too super(Values, self).__init__(storage) - values_table = 'CREATE TABLE IF NOT EXISTS value(path TEXT, ' - values_table += 'value TEXT, owner TEXT, idx INTEGER, session_id TEXT NOT NULL, '\ - 'PRIMARY KEY (path, idx, session_id))' - self._storage.execute(values_table, commit=False) - informations_table = 'CREATE TABLE IF NOT EXISTS information(key TEXT PRIMARY ' - informations_table += 'KEY, value TEXT, session_id TEXT NOT NULL)' - self._storage.execute(informations_table) def getsession(self): pass # sqlite def _sqlite_select(self, path, index): - request = "SELECT value FROM value WHERE path = ? AND session_id = ?" + request = "SELECT value FROM value WHERE path = ? AND session_id = ? " params = (path, self._session_id) if index is not None: - request += " and idx = ?" + request += "and idx = ? " params = (path, self._session_id, index) + request += "LIMIT 1" return self._storage.select(request, params) # value @@ -62,7 +55,7 @@ class Values(Sqlite3DB): commit=False) self._storage.execute("INSERT INTO value(path, value, owner, idx, session_id) VALUES " "(?, ?, ?, ?, ?)", (path, self._sqlite_encode(value), - self._sqlite_encode(str(owner)), + str(owner), index, self._session_id), commit=True) @@ -72,7 +65,7 @@ class Values(Sqlite3DB): commit=False) self._storage.execute("INSERT INTO value(path, value, owner, session_id) VALUES " "(?, ?, ?, ?)", (path, self._sqlite_encode(value), - self._sqlite_encode(str(owner)), + str(owner), self._session_id), commit=True) @@ -109,7 +102,7 @@ class Values(Sqlite3DB): (self._session_id,), only_one=False): path = self._sqlite_decode_path(path) - owner = getattr(owners, self._sqlite_decode(owner)) + owner = getattr(owners, owner) value = self._sqlite_decode(value) if isinstance(value, list): @@ -131,33 +124,41 @@ class Values(Sqlite3DB): path = self._sqlite_encode_path(path) if index is None: self._storage.execute("UPDATE value SET owner = ? WHERE path = ? AND session_id = ?", - (self._sqlite_encode(str(owner)), path, self._session_id)) + (str(owner), path, self._session_id)) else: self._storage.execute("UPDATE value SET owner = ? WHERE path = ? and idx = ? AND session_id = ?", - (self._sqlite_encode(str(owner)), path, index, self._session_id)) + (str(owner), path, index, self._session_id)) - def getowner(self, path, default, session, index=None, only_default=False): + def getowner(self, path, default, session, index=None, only_default=False, with_value=False): """get owner for an option return: owner object """ path = self._sqlite_encode_path(path) - request = "SELECT owner FROM value WHERE path = ? AND session_id = ?" + request = "SELECT owner, value FROM value WHERE path = ? AND session_id = ?" if index is not None: request += " AND idx = ?" params = (path, self._session_id, index) else: params = (path, self._session_id) + request += ' LIMIT 1' owner = self._storage.select(request, params) if owner is None: - return default + if not with_value: + return default + else: + return default, None else: - owner = self._sqlite_decode(owner[0]) # autocreate owners try: - return getattr(owners, owner) + nowner = getattr(owners, owner[0]) except AttributeError: - owners.addowner(owner) - return getattr(owners, owner) + owners.addowner(towner) + nowner = getattr(owners, owner[0]) + if not with_value: + return nowner + else: + value = self._sqlite_decode(owner[1]) + return nowner, value def set_information(self, key, value): """updates the information's attribute @@ -203,7 +204,7 @@ class Values(Sqlite3DB): for row in rows: path = row[0] value = self._sqlite_decode(row[1]) - owner = self._sqlite_decode(row[2]) + owner = row[2] index = row[3] if index is None: ret[0].append(path) @@ -232,7 +233,7 @@ class Values(Sqlite3DB): owner = export[3][idx] self._storage.execute("INSERT INTO value(path, value, owner, idx, session_id) VALUES " "(?, ?, ?, ?, ?)", (path, self._sqlite_encode(value), - self._sqlite_encode(str(owner)), index, + str(owner), index, self._session_id)) self._storage._conn.commit() diff --git a/tiramisu/value.py b/tiramisu/value.py index 82e4ce9..fa28747 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -129,19 +129,17 @@ class Values(object): _index = None else: _index = index - is_default = self._p_.getowner(path, owners.default, session, only_default=True, index=_index) == owners.default + owner, value = self._p_.getowner(path, owners.default, session, only_default=True, + index=_index, with_value=True) + is_default = owner == owners.default if not is_default and not force_default: - if opt.impl_is_master_slaves('slave'): - return self._p_.getvalue(path, session, index) + if index is not None and not opt.impl_is_master_slaves('slave'): + if len(value) > index: + return value[index] + #value is smaller than expected + #so return default value else: - value = self._p_.getvalue(path, session) - if index is not None: - if len(value) > index: - return value[index] - #value is smaller than expected - #so return default value - else: - return value + return value return self._getdefaultvalue(opt, path, with_meta, index, submulti_index, validate) @@ -364,7 +362,8 @@ class Values(object): force_index=index, force_submulti_index=force_submulti_index, display_error=True, - display_warnings=False) + display_warnings=False, + setting_properties=setting_properties) if err: config_error = err value = None @@ -391,7 +390,7 @@ class Values(object): force_submulti_index=force_submulti_index, display_error=False, display_warnings=display_warnings, - setting_properties=setting_properties,) + setting_properties=setting_properties) if config_error is not None: return config_error return value @@ -400,13 +399,15 @@ class Values(object): raise ConfigError(_('you should only set value with config')) def setitem(self, opt, value, path, force_permissive=False, - check_frozen=True, not_raises=False, index=None): + check_frozen=True, not_raises=False, index=None, + _setting_properties=undefined): # check_frozen is, for example, used with "force_store_value" # user didn't change value, so not write # valid opt context = self._getcontext() - setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=False) - if 'validator' in setting_properties: + if _setting_properties is undefined: + _setting_properties = context.cfgimpl_get_settings()._getproperties(read_write=False) + if 'validator' in _setting_properties: session = context.cfgimpl_get_values()._p_.getsession() fake_context = context._gen_fake_values(session) fake_values = fake_context.cfgimpl_get_values() @@ -414,15 +415,17 @@ class Values(object): props = fake_values.validate(opt, value, path, check_frozen=check_frozen, force_permissive=force_permissive, - setting_properties=setting_properties, + setting_properties=_setting_properties, session=session, not_raises=not_raises, index=index) if props and not_raises: return - err = opt.impl_validate(value, fake_context, display_warnings=False, force_index=index) + err = opt.impl_validate(value, fake_context, display_warnings=False, force_index=index, + setting_properties=_setting_properties) if err: raise err - opt.impl_validate(value, fake_context, display_error=False) + opt.impl_validate(value, fake_context, display_error=False, + setting_properties=_setting_properties) self._setvalue(opt, path, value, index=index) def _setvalue(self, opt, path, value, force_owner=undefined, index=None):