From f6f709f83db0190954de494093da1a05f1968f05 Mon Sep 17 00:00:00 2001
From: Emmanuel Garette <egarette@cadoles.com>
Date: Wed, 14 Aug 2013 23:06:31 +0200
Subject: [PATCH] split cache/value/setting in plugin

---
 tiramisu/config.py                      |   6 +-
 tiramisu/plugins/__init__.py            |   0
 tiramisu/plugins/dictionary/__init__.py |   0
 tiramisu/plugins/dictionary/cache.py    |  48 ++++++++++
 tiramisu/plugins/dictionary/setting.py  |  58 ++++++++++++
 tiramisu/plugins/dictionary/value.py    |  76 +++++++++++++++
 tiramisu/setting.py                     |  99 ++++++++-----------
 tiramisu/value.py                       | 121 +++++++++++++-----------
 8 files changed, 290 insertions(+), 118 deletions(-)
 create mode 100644 tiramisu/plugins/__init__.py
 create mode 100644 tiramisu/plugins/dictionary/__init__.py
 create mode 100644 tiramisu/plugins/dictionary/cache.py
 create mode 100644 tiramisu/plugins/dictionary/setting.py
 create mode 100644 tiramisu/plugins/dictionary/value.py

diff --git a/tiramisu/config.py b/tiramisu/config.py
index 510d489..bd71d9b 100644
--- a/tiramisu/config.py
+++ b/tiramisu/config.py
@@ -23,7 +23,7 @@
 from tiramisu.error import PropertiesOptionError, ConfigError
 from tiramisu.option import OptionDescription, Option, SymLinkOption, \
     BaseInformation
-from tiramisu.setting import groups, Setting, default_encoding
+from tiramisu.setting import groups, Settings, default_encoding
 from tiramisu.value import Values
 from tiramisu.i18n import _
 
@@ -505,7 +505,7 @@ class Config(CommonConfig):
         :param context: the current root config
         :type context: `Config`
         """
-        self._impl_settings = Setting(self)
+        self._impl_settings = Settings(self)
         self._impl_values = Values(self)
         super(Config, self).__init__(descr, self)
         self._impl_build_all_paths()
@@ -541,7 +541,7 @@ class MetaConfig(CommonConfig):
                 child._impl_meta = self
 
         self._impl_children = children
-        self._impl_settings = Setting(self)
+        self._impl_settings = Settings(self)
         self._impl_values = Values(self)
         self._impl_meta = None
         self._impl_informations = {}
diff --git a/tiramisu/plugins/__init__.py b/tiramisu/plugins/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tiramisu/plugins/dictionary/__init__.py b/tiramisu/plugins/dictionary/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tiramisu/plugins/dictionary/cache.py b/tiramisu/plugins/dictionary/cache.py
new file mode 100644
index 0000000..fd67847
--- /dev/null
+++ b/tiramisu/plugins/dictionary/cache.py
@@ -0,0 +1,48 @@
+# -*- coding: utf-8 -*-
+"default plugin for cache: set it in a simple dictionary"
+# Copyright (C) 2013 Team tiramisu (see AUTHORS for all contributors)
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# ____________________________________________________________
+
+
+class PluginCache(object):
+    __slots__ = ('_cache',)
+
+    def __init__(self):
+        self._cache = {}
+
+    def _p_setcache(self, cache_type, opt, val, time):
+        self._cache[opt] = (val, time)
+
+    def _p_getcache(self, cache_type, opt, exp):
+        value, created = self._cache[opt]
+        if exp < created:
+            return True, value
+        return False, None
+
+    def _p_hascache(self, cache_type, opt):
+        return opt in self._cache
+
+    def _p_reset_expired_cache(self, cache_type, exp):
+        keys = self._cache.keys()
+        for key in keys:
+            val, created = self._cache[key]
+            if exp > created:
+                del(self._cache[key])
+
+    def _p_reset_all_cache(self, cache_type):
+        self._cache.clear()
diff --git a/tiramisu/plugins/dictionary/setting.py b/tiramisu/plugins/dictionary/setting.py
new file mode 100644
index 0000000..3ccbeb1
--- /dev/null
+++ b/tiramisu/plugins/dictionary/setting.py
@@ -0,0 +1,58 @@
+# -*- coding: utf-8 -*-
+"default plugin for setting: set it in a simple dictionary"
+# Copyright (C) 2013 Team tiramisu (see AUTHORS for all contributors)
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# ____________________________________________________________
+from tiramisu.plugins.dictionary.cache import PluginCache
+
+
+class PluginSettings(PluginCache):
+    __slots__ = ('_properties', '_permissives')
+
+    def __init__(self):
+        # properties attribute: the name of a property enables this property
+        # key is None for global properties
+        self._properties = {}
+        # permissive properties
+        self._permissives = {}
+        super(PluginSettings, self).__init__()
+
+    # propertives
+    def _p_setproperties(self, opt, properties):
+        self._properties[opt] = properties
+
+    def _p_getproperties(self, opt, default_properties):
+        return self._properties.get(opt, set(default_properties))
+
+    def _p_hasproperties(self, opt):
+        return opt in self._properties
+
+    def _p_reset_all_propertives(self):
+        self._properties = {}
+
+    def _p_reset_properties(self, opt):
+        try:
+            del(self._properties[opt])
+        except KeyError:
+            pass
+
+    # permissive
+    def _p_setpermissive(self, opt, permissive):
+        self._permissives[opt] = frozenset(permissive)
+
+    def _p_getpermissive(self, opt=None):
+        return self._permissives.get(opt, frozenset())
diff --git a/tiramisu/plugins/dictionary/value.py b/tiramisu/plugins/dictionary/value.py
new file mode 100644
index 0000000..84f60c2
--- /dev/null
+++ b/tiramisu/plugins/dictionary/value.py
@@ -0,0 +1,76 @@
+# -*- coding: utf-8 -*-
+"default plugin for value: set it in a simple dictionary"
+# Copyright (C) 2013 Team tiramisu (see AUTHORS for all contributors)
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# ____________________________________________________________
+
+#FIXME
+from tiramisu.plugins.dictionary.cache import PluginCache
+
+
+class PluginValues(PluginCache):
+    __slots__ = ('_values',)
+
+    def __init__(self):
+        """init plugin means create values storage
+        """
+        self._values = {}
+        # should init cache too
+        super(PluginValues, self).__init__()
+
+    # value
+    def _p_setvalue(self, opt, value):
+        """set value for an option
+        a specified value must be associated to an owner
+        """
+        owner = self.context.cfgimpl_get_settings().getowner()
+        self._values[opt] = (owner, value)
+
+    def _p_getvalue(self, opt):
+        """get value for an option
+        return: only value, not the owner
+        """
+        return self._values[opt][1]
+
+    def _p_hasvalue(self, opt):
+        """if opt has a value
+        return: boolean
+        """
+        return opt in self._values
+
+    def _p_resetvalue(self, opt):
+        """remove value means delete value in storage
+        """
+        del(self._values[opt])
+
+    def _p_get_modified_values(self):
+        """return all values in a dictionary
+        example: {option1: (owner, 'value1'), option2: (owner, 'value2')}
+        """
+        return self._values
+
+    # owner
+    def _p_setowner(self, opt, owner):
+        """change owner for an option
+        """
+        self._values[opt] = (owner, self._values[opt][1])
+
+    def _p_getowner(self, opt, default):
+        """get owner for an option
+        return: owner object
+        """
+        return self._values.get(opt, (default, None))[0]
diff --git a/tiramisu/setting.py b/tiramisu/setting.py
index ecfc5bf..43e2d70 100644
--- a/tiramisu/setting.py
+++ b/tiramisu/setting.py
@@ -24,6 +24,8 @@ from time import time
 from copy import copy
 from tiramisu.error import RequirementError, PropertiesOptionError
 from tiramisu.i18n import _
+from tiramisu.plugins.dictionary.setting import PluginSettings
+
 
 default_encoding = 'utf-8'
 expires_time = 5
@@ -158,12 +160,12 @@ class Property(object):
 
     def append(self, propname):
         self._properties.add(propname)
-        self._setting._set_properties(self._properties, self._opt)
+        self._setting._setproperties(self._properties, self._opt)
 
     def remove(self, propname):
         if propname in self._properties:
             self._properties.remove(propname)
-            self._setting._set_properties(self._properties, self._opt)
+            self._setting._setproperties(self._properties, self._opt)
 
     def reset(self):
         self._setting.reset(opt=self._opt)
@@ -176,31 +178,26 @@ class Property(object):
 
 
 #____________________________________________________________
-class Setting(object):
+class Settings(PluginSettings):
     "``Config()``'s configuration options"
-    __slots__ = ('context', '_properties', '_permissives', '_owner', '_cache')
+    __slots__ = ('context', '_owner')
 
     def __init__(self, context):
-        # properties attribute: the name of a property enables this property
-        # key is None for global properties
-        self._properties = {}
-        # permissive properties
-        self._permissives = {}
         # generic owner
         self._owner = owners.user
         self.context = context
-        self._cache = {}
+        super(Settings, self).__init__()
 
     #____________________________________________________________
     # properties methods
     def __contains__(self, propname):
-        return propname in self._get_properties()
+        return propname in self._getproperties()
 
     def __repr__(self):
-        return str(list(self._get_properties()))
+        return str(list(self._getproperties()))
 
     def __getitem__(self, opt):
-        return Property(self, self._get_properties(opt), opt)
+        return Property(self, self._getproperties(opt), opt)
 
     def __setitem__(self, opt, value):
         raise ValueError('you must only append/remove properties')
@@ -210,50 +207,49 @@ class Setting(object):
             raise ValueError(_('opt and all_properties must not be set '
                                'together in reset'))
         if all_properties:
-            self._properties = {}
+            self._p_reset_all_propertives()
         else:
-            try:
-                del(self._properties[opt])
-            except KeyError:
-                pass
+            self._p_reset_properties(opt)
         self.context.cfgimpl_reset_cache()
 
-    def _get_properties(self, opt=None, is_apply_req=True):
+    def _getproperties(self, opt=None, is_apply_req=True):
         if opt is None:
-            props = self._properties.get(opt, set(default_properties))
+            props = self._p_getproperties(opt, default_properties)
         else:
-            exp = None
-            if opt in self._cache:
-                exp = time()
-                props, created = self._cache[opt]
-                if exp < created:
+            ntime = None
+            if self._p_hascache('properties', opt):
+                ntime = time()
+                is_cached, props = self._p_getcache('properties', opt, ntime)
+                if is_cached:
                     return props
             if is_apply_req:
                 self.apply_requires(opt)
-            props = self._properties.get(opt, set(opt._properties))
-            self._set_cache(opt, props, exp)
+            props = self._p_getproperties(opt, opt._properties)
+            if 'expire' in self:
+                if ntime is None:
+                    ntime = time()
+                self._p_setcache('properties', opt, props, ntime + expires_time)
         return props
 
     def append(self, propname):
         "puts property propname in the Config's properties attribute"
-        Property(self, self._get_properties()).append(propname)
+        Property(self, self._getproperties()).append(propname)
 
     def remove(self, propname):
         "deletes property propname in the Config's properties attribute"
-        Property(self, self._get_properties()).remove(propname)
+        Property(self, self._getproperties()).remove(propname)
 
-    def _set_properties(self, properties, opt=None):
+    def _setproperties(self, properties, opt=None):
         """save properties for specified opt
         (never save properties if same has option properties)
         """
         if opt is None:
-            self._properties[opt] = properties
+            self._p_setproperties(opt, properties)
         else:
             if set(opt._properties) == properties:
-                if opt in self._properties:
-                    del(self._properties[opt])
+                self._p_reset_properties(opt)
             else:
-                self._properties[opt] = properties
+                self._p_setproperties(opt, properties)
         self.context.cfgimpl_reset_cache()
 
     #____________________________________________________________
@@ -261,13 +257,13 @@ class Setting(object):
                             value=None, force_permissive=False,
                             force_properties=None):
         #opt properties
-        properties = copy(self._get_properties(opt_or_descr))
+        properties = copy(self._getproperties(opt_or_descr))
         #remove opt permissive
-        properties -= self._get_permissive(opt_or_descr)
+        properties -= self._p_getpermissive(opt_or_descr)
         #remove global permissive if need
-        self_properties = copy(self._get_properties())
+        self_properties = copy(self._getproperties())
         if force_permissive is True or 'permissive' in self_properties:
-            properties -= self._get_permissive()
+            properties -= self._p_getpermissive()
 
         #global properties
         if force_properties is not None:
@@ -280,8 +276,8 @@ class Setting(object):
             properties -= frozenset(('mandatory', 'frozen'))
         else:
             if 'mandatory' in properties and \
-                    not self.context.cfgimpl_get_values()._is_empty(opt_or_descr,
-                                                                    value):
+                    not self.context.cfgimpl_get_values()._isempty(opt_or_descr,
+                                                                   value):
                 properties.remove('mandatory')
             if is_write and 'everything_frozen' in self_properties:
                 properties.add('frozen')
@@ -299,13 +295,11 @@ class Setting(object):
                                               "named: {0} with properties {1}"
                                               "").format(opt_or_descr._name, str(props)), props)
 
-    def _get_permissive(self, opt=None):
-        return self._permissives.get(opt, frozenset())
-
+    #FIXME should be setpermissive
     def set_permissive(self, permissive, opt=None):
         if not isinstance(permissive, tuple):
             raise TypeError(_('permissive must be a tuple'))
-        self._permissives[opt] = frozenset(permissive)
+        self._p_setpermissive(opt, permissive)
 
     #____________________________________________________________
     def setowner(self, owner):
@@ -332,22 +326,11 @@ class Setting(object):
         "convenience method to freeze, hidde and disable"
         self._read(rw_remove, rw_append)
 
-    def _set_cache(self, opt, props, exp):
-        if 'expire' in self:
-            if exp is None:
-                exp = time()
-            self._cache[opt] = (props, time() + expires_time)
-
     def reset_cache(self, only_expired):
         if only_expired:
-            exp = time()
-            keys = self._cache.keys()
-            for key in keys:
-                props, created = self._cache[key]
-                if exp > created:
-                    del(self._cache[key])
+            self._p_reset_expired_cache('properties', time())
         else:
-            self._cache.clear()
+            self._p_reset_all_cache('properties')
 
     def apply_requires(self, opt):
         "carries out the jit (just in time requirements between options"
@@ -355,7 +338,7 @@ class Setting(object):
             return
 
         # filters the callbacks
-        setting = Property(self, self._get_properties(opt, False), opt)
+        setting = Property(self, self._getproperties(opt, False), opt)
         descr = self.context.cfgimpl_get_description()
         optpath = descr.impl_get_path_by_opt(opt)
         for requires in opt._requires:
diff --git a/tiramisu/value.py b/tiramisu/value.py
index 65a23b1..d22bec3 100644
--- a/tiramisu/value.py
+++ b/tiramisu/value.py
@@ -25,13 +25,16 @@ from tiramisu.autolib import carry_out_calculation
 from tiramisu.i18n import _
 from tiramisu.option import SymLinkOption
 
+#FIXME
+from tiramisu.plugins.dictionary.value import PluginValues
 
-class Values(object):
+
+class Values(PluginValues):
     """The `Config`'s root is indeed  in charge of the `Option()`'s values,
     but the values are physicaly located here, in `Values`, wich is also
     responsible of a caching utility.
     """
-    __slots__ = ('context', '_values', '_cache')
+    __slots__ = ('context',)
 
     def __init__(self, context):
         """
@@ -42,11 +45,9 @@ class Values(object):
         """
 
         self.context = context
-        self._values = {}
-        self._cache = {}
         super(Values, self).__init__()
 
-    def _get_default(self, opt):
+    def _getdefault(self, opt):
         meta = self.context.cfgimpl_get_meta()
         if meta is not None:
             value = meta.cfgimpl_get_values()[opt]
@@ -57,33 +58,42 @@ class Values(object):
         else:
             return value
 
-    def _get_value(self, opt, validate=True):
+    def _getvalue(self, opt, validate=True):
         "return value or default value if not set"
-        #if no value
-        if opt not in self._values:
-            value = self._get_default(opt)
+        if not self._p_hasvalue(opt):
+            #if no value
+            value = self._getdefault(opt)
             if opt.impl_is_multi():
                 value = Multi(value, self.context, opt, validate)
         else:
             #if value
-            value = self._values[opt][1]
+            value = self._p_getvalue(opt)
+            if opt.impl_is_multi() and not isinstance(value, Multi):
+                #load value so don't need to validate if is not a Multi
+                value = Multi(value, self.context, opt, validate=False)
         return value
 
-    def __delitem__(self, opt):
-        self._reset(opt)
+    def get_modified_values(self):
+        return self._p_get_modified_values()
 
-    def _reset(self, opt):
-        if opt in self._values:
+    def __contains__(self, opt):
+        return self._p_hascache('value', opt)
+
+    def __delitem__(self, opt):
+        self.reset(opt)
+
+    def reset(self, opt):
+        if self._p_hasvalue(opt):
             setting = self.context.cfgimpl_get_settings()
             opt.impl_validate(opt.impl_getdefault(), self.context,
                               'validator' in setting)
             self.context.cfgimpl_reset_cache()
             if opt.impl_is_multi() and opt.impl_get_multitype() == multitypes.master:
                 for slave in opt.impl_get_master_slaves():
-                    self._reset(slave)
-            del(self._values[opt])
+                    self.reset(slave)
+            self._p_resetvalue(opt)
 
-    def _is_empty(self, opt, value):
+    def _isempty(self, opt, value):
         "convenience method to know if an option is empty"
         empty = opt._empty
         if (not opt.impl_is_multi() and (value is None or value == empty)) or \
@@ -104,21 +114,23 @@ class Values(object):
     def __getitem__(self, opt):
         return self.getitem(opt)
 
-    def get_modified_values(self):
-        return self._values
-
     def getitem(self, opt, validate=True, force_permissive=False,
                 force_properties=None, validate_properties=True):
-        if opt in self._cache:
-            exp = time()
-            value, created = self._cache[opt]
-            if exp < created:
+        ntime = None
+        if self._p_hascache('value', opt):
+            ntime = time()
+            is_cached, value = self._p_getcache('value', opt, ntime)
+            if is_cached:
                 return value
         val = self._getitem(opt, validate, force_permissive, force_properties,
                             validate_properties)
-        if validate and validate_properties and force_permissive is False and \
+        if 'expire' in self.context.cfgimpl_get_settings() and validate and \
+                validate_properties and force_permissive is False and \
                 force_properties is None:
-            self._set_cache(opt, val)
+            if ntime is None:
+                ntime = time()
+            self._p_setcache('value', opt, val, ntime + expires_time)
+
         return val
 
     def _getitem(self, opt, validate, force_permissive, force_properties,
@@ -148,14 +160,14 @@ class Values(object):
             if opt.impl_is_multi():
                 value = Multi(value, self.context, opt, validate)
             #suppress value if already set
-            self._reset(opt)
+            self.reset(opt)
         # frozen and force default
         elif is_frozen and 'force_default_on_freeze' in setting[opt]:
-            value = self._get_default(opt)
+            value = self._getdefault(opt)
             if opt.impl_is_multi():
                 value = Multi(value, self.context, opt, validate)
         else:
-            value = self._get_value(opt, validate)
+            value = self._getvalue(opt, validate)
         if validate:
             opt.impl_validate(value, self.context, 'validator' in setting)
         if self.is_default_owner(opt) and \
@@ -184,29 +196,30 @@ class Values(object):
     def _setvalue(self, opt, value, force_permissive=False, force_properties=None,
                   is_write=True, validate_properties=True):
         self.context.cfgimpl_reset_cache()
-        setting = self.context.cfgimpl_get_settings()
         if validate_properties:
+            setting = self.context.cfgimpl_get_settings()
             setting.validate_properties(opt, False, is_write,
                                         value=value,
                                         force_permissive=force_permissive,
                                         force_properties=force_properties)
-        self._values[opt] = (setting.getowner(), value)
+        self._p_setvalue(opt, value)
 
     def getowner(self, opt):
         if isinstance(opt, SymLinkOption):
             opt = opt._opt
-        owner = self._values.get(opt, (owners.default, None))[0]
+        owner = self._p_getowner(opt, owners.default)
         meta = self.context.cfgimpl_get_meta()
         if owner is owners.default and meta is not None:
             owner = meta.cfgimpl_get_values().getowner(opt)
         return owner
 
     def setowner(self, opt, owner):
-        if opt not in self._values:
-            raise ConfigError(_('no value for {0} cannot change owner to {1}').format(opt._name, owner))
         if not isinstance(owner, owners.Owner):
             raise TypeError(_("invalid generic owner {0}").format(str(owner)))
-        self._values[opt] = (owner, self._values[opt][1])
+        if self.getowner(opt) == owners.default:
+            raise ConfigError(_('no value for {0} cannot change owner to {1}'
+                                '').format(opt._name, owner))
+        self._p_setowner(opt, owner)
 
     def is_default_owner(self, opt):
         """
@@ -216,23 +229,17 @@ class Values(object):
         """
         return self.getowner(opt) == owners.default
 
-    def _set_cache(self, opt, val):
-        if 'expire' in self.context.cfgimpl_get_settings():
-            self._cache[opt] = (val, time() + expires_time)
-
     def reset_cache(self, only_expired):
         if only_expired:
-            exp = time()
-            keys = self._cache.keys()
-            for key in keys:
-                val, created = self._cache[key]
-                if exp > created:
-                    del(self._cache[key])
+            self._p_reset_expired_cache('value', time())
         else:
-            self._cache.clear()
+            self._p_reset_all_cache('value')
 
-    def __contains__(self, opt):
-        return opt in self._values
+    def _get_opt_path(self, opt):
+        return self.context.cfgimpl_get_description().impl_get_path_by_opt(opt)
+
+    def _get_owner_from_str(self, owner):
+        return getattr(owners, owner)
 
 # ____________________________________________________________
 # multi types
@@ -281,7 +288,7 @@ class Multi(list):
         values = self.context.cfgimpl_get_values()
         for slave in self.opt._master_slaves:
             if not values.is_default_owner(slave):
-                value_slave = values._get_value(slave)
+                value_slave = values._getvalue(slave)
                 if len(value_slave) > masterlen:
                     raise SlaveError(_("invalid len for the master: {0}"
                                        " which has {1} as slave with"
@@ -294,8 +301,8 @@ class Multi(list):
     def __setitem__(self, key, value):
         self._validate(value)
         #assume not checking mandatory property
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
         super(Multi, self).__setitem__(key, value)
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
 
     def append(self, value, force=False):
         """the list value can be updated (appened)
@@ -313,8 +320,6 @@ class Multi(list):
                     if isinstance(value, list):
                         value = None
         self._validate(value)
-        #set value without valid properties
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self, validate_properties=not force)
         super(Multi, self).append(value)
         if not force and self.opt.impl_get_multitype() == multitypes.master:
             for slave in self.opt.impl_get_master_slaves():
@@ -327,38 +332,39 @@ class Multi(list):
                     #get multi without valid properties
                     values.getitem(slave, validate_properties=False).append(
                         dvalue, force=True)
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self, validate_properties=not force)
 
     def sort(self, cmp=None, key=None, reverse=False):
         if self.opt.impl_get_multitype() in [multitypes.slave,
                                              multitypes.master]:
             raise SlaveError(_("cannot sort multi option {0} if master or slave"
                                "").format(self.opt._name))
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
         super(Multi, self).sort(cmp=cmp, key=key, reverse=reverse)
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
 
     def reverse(self):
         if self.opt.impl_get_multitype() in [multitypes.slave,
                                              multitypes.master]:
             raise SlaveError(_("cannot reverse multi option {0} if master or "
                                "slave").format(self.opt._name))
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
         super(Multi, self).reverse()
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
 
     def insert(self, index, obj):
         if self.opt.impl_get_multitype() in [multitypes.slave,
                                              multitypes.master]:
             raise SlaveError(_("cannot insert multi option {0} if master or "
                                "slave").format(self.opt._name))
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
         super(Multi, self).insert(index, obj)
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
 
     def extend(self, iterable):
         if self.opt.impl_get_multitype() in [multitypes.slave,
                                              multitypes.master]:
             raise SlaveError(_("cannot extend multi option {0} if master or "
                                "slave").format(self.opt._name))
-        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
         super(Multi, self).extend(iterable)
+        self.context.cfgimpl_get_values()._setvalue(self.opt, self)
 
     def _validate(self, value):
         if value is not None:
@@ -387,5 +393,6 @@ class Multi(list):
                         #get multi without valid properties
                         values.getitem(slave, validate_properties=False).pop(key, force=True)
         #set value without valid properties
+        ret = super(Multi, self).pop(key)
         self.context.cfgimpl_get_values()._setvalue(self.opt, self, validate_properties=not force)
-        return super(Multi, self).pop(key)
+        return ret