Merge pull request #160 from jbasko/track-changes2

Track changes fixes and reset_changes() added

Author: jbasko

.bumpversion.cfg

@@ -1,5 +1,5 @@
 [bumpversion]
-current_version = 1.28.0
+current_version = 1.29.0
 commit = true
 tag = false
 

configmanager/__init__.py

@@ -1,4 +1,4 @@
-__version__ = '1.28.0'
+__version__ = '1.29.0'
 
 from .managers import Config
 from .items import Item

configmanager/items.py

@@ -204,23 +204,62 @@ def get(self, fallback=not_set):
             raise RequiredValueMissing(name=self.name, item=self)
         return fallback
 
+    """
+    Design comment:
+    
+    It is by design that we report not_set as old_value when value is set for the first time,
+    and not_set as new_value when item is reset.
+    This is because reporting effective value would cause inconsistency in what is reported.
+    Listeners of item_value_changed can always look into item.default or item.value.
+    """
+
     def set(self, value):
         """
         Sets config value.
         """
         old_value = self._value
+
         self.type.set_item_value(self, value)
+
         new_value = self._value
+
+        if old_value is not_set and new_value is not_set:
+            # Nothing to report
+            return
+
         if self.section:
-            self.section._trigger_event(self.section.hooks.item_value_changed, item=self, old_value=old_value, new_value=new_value)
+            self.section._trigger_event(
+                self.section.hooks.item_value_changed,
+                item=self,
+                old_value=old_value,
+                new_value=new_value,
+            )
 
     def reset(self):
         """
         Resets the value of config item to its default value.
         """
+        old_value = self._value
+
         self._value = not_set
         self.raw_str_value = not_set
 
+        new_value = self._value
+
+        if old_value is not_set:
+            # Nothing to report
+            return
+
+        if self.section:
+            self.section._trigger_event(
+                self.section.hooks.item_value_changed,
+                item=self,
+                old_value=old_value,
+                new_value=new_value,
+            )
+
+
+
     @property
     def is_default(self):
         """

configmanager/managers.py

@@ -1,3 +1,5 @@
+import collections
+
 from .utils import _get_persistence_adapter_for
 from .schema_parser import parse_config_schema
 from .meta import ConfigManagerSettings
@@ -9,7 +11,7 @@ class _TrackingContext(object):
     def __init__(self, config):
         self.config = config
         self.hook = None
-        self.changes = {}
+        self._changes = collections.defaultdict(list)
 
     def __enter__(self):
         return self.push()
@@ -19,7 +21,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
 
     def _value_changed(self, item, old_value, new_value):
         if old_value != new_value:
-            self.changes[item] = new_value
+            self._changes[item].append((old_value, new_value))
 
     def push(self):
         assert self.hook is None
@@ -32,7 +34,28 @@ def pop(self):
         assert popped is self
         self.config.hooks.unregister_hook(self.config.hooks.item_value_changed, self._value_changed)
         self.hook = None
-        return self.changes
+
+    @property
+    def changes(self):
+        values = {}
+        for k, k_changes in self._changes.items():
+            if len(k_changes) == 1:
+                values[k] = k_changes[0][1]
+            elif k_changes[0][0] != k_changes[-1][1]:
+                values[k] = k_changes[-1][1]
+        return values
+
+    def reset_changes(self, item=None):
+
+        for k, k_changes in self._changes.items():
+            if item is None or k is item:
+                # TODO This doesn't reset raw_str_value properly ...
+                k._value = k_changes[0][0]
+
+        if item is None:
+            self._changes.clear()
+        else:
+            del self._changes[item]
 
 
 class Config(Section):

configmanager/sections.py

@@ -255,6 +255,10 @@ def get_proxy(self, *key):
 
     @property
     def hooks(self):
+        """
+        Returns:
+            HookRegistry
+        """
         return self._hooks
 
     @property

docs/index.rst

@@ -325,3 +325,17 @@ to be called when this exception is raised:
 
 
 If this function returns anything other than ``None``, the exception will not be raised.
+
+How can I track changes of config values?
+-----------------------------------------
+
+.. code-block:: python
+
+    >>> with config.tracking_context() as ctx:
+    ...     config.greeting.value = 'Hey, what is up!'
+
+    >>> len(ctx.changes)
+    1
+
+    >>> ctx.changes[config.greeting]
+    'Hey, what is up!'

tests/test_hooks.py

@@ -1,6 +1,6 @@
 import pytest
 
-from configmanager import Config, NotFound, Section
+from configmanager import Config, NotFound, Section, Item
 from configmanager.utils import not_set
 
 
@@ -255,6 +255,68 @@ def item_value_changed(old_value=None, new_value=None, item=None, **kwargs):
     assert calls[-1] == (config.uploads.db.user, 'Administrator', 'NEW VALUE')
 
 
+def test_item_value_changed_reports_not_set_as_old_value_if_there_was_no_value_before():
+    config = Config({'a': 'aaa'})
+    calls = []
+
+    def first(old_value, new_value):
+        assert old_value is not_set
+        assert new_value == 'bbb'
+        calls.append(1)
+
+    def second(old_value, new_value):
+        assert old_value == 'bbb'
+        assert new_value == 'aaa'
+        calls.append(2)
+
+    config.hooks.register_hook('item_value_changed', first)
+    config.a.value = 'bbb'
+    config.hooks.unregister_hook('item_value_changed', first)
+
+    config.hooks.register_hook('item_value_changed', second)
+    config.a.value = 'aaa'
+    config.hooks.unregister_hook('item_value_changed', second)
+
+    assert calls == [1, 2]
+
+
+def test_item_value_changed_hook_called_on_item_reset():
+    config = Config({'a': 'aaa', 'b': 'bbb', 'c': Item()})
+    calls = []
+
+    @config.hooks.item_value_changed
+    def item_value_changed(item, old_value, new_value):
+        calls.append(item.name)
+
+    assert len(calls) == 0
+
+    config.reset()
+    assert len(calls) == 0
+
+    # Setting same value as default value triggers the event
+    config.a.value = 'aaa'
+    assert calls == ['a']
+
+    # Setting same value as the custom value before triggers the event
+    config.a.value = 'aaa'
+    assert calls == ['a', 'a']
+
+    # Actual reset
+    config.reset()
+    assert calls == ['a', 'a', 'a']
+
+
+def test_item_value_changed_hook_not_called_when_resetting_a_not_set():
+    config = Config({'a': Item()})
+
+    @config.hooks.item_value_changed
+    def item_value_changed(item, old_value, new_value):
+        raise AssertionError('This should not have been called')
+
+    config.reset()
+    config.a.value = not_set
+
+
 def test_hooks_arent_handled_if_hooks_enabled_setting_is_set_to_falsey_value():
     config = Config({
         'uploads': {