Fix container items change event being saved in preferences

apptools/preferences/preferences_helper.py

@@ -17,7 +17,14 @@
 
 
 class PreferencesHelper(HasTraits):
-    """ An object that can be initialized from a preferences node. """
+    """ A base class for objects that can be initialized from a preferences
+    node.
+
+    Additional traits defined on subclasses will be listened to. Changes
+    are then synchronized with the preferences. Note that mutations on nested
+    containers e.g. List(List(Str)) cannot be synchronized and should be
+    avoided.
+    """
 
     #### 'PreferencesHelper' interface ########################################
 
@@ -65,9 +72,23 @@ def _preferences_default(self):
     def _anytrait_changed(self, trait_name, old, new):
         """ Static trait change handler. """
 
+        if self.preferences is None:
+            return
+
+        # If the trait was a list or dict '_items' trait then just treat it as
+        # if the entire list or dict was changed.
+        if trait_name.endswith('_items'):
+            trait_name = trait_name[:-6]
+            if self._is_preference_trait(trait_name):
+                self.preferences.set(
+                    '%s.%s' % (self._get_path(), trait_name),
+                    getattr(self, trait_name)
+                )
+                return
+
         # If we were the one that set the trait (because the underlying
         # preferences node changed) then do nothing.
-        if self.preferences and self._is_preference_trait(trait_name):
+        if self._is_preference_trait(trait_name):
             self.preferences.set("%s.%s" % (self._get_path(), trait_name), new)
 
         return

apptools/preferences/tests/test_preferences_helper.py

@@ -15,7 +15,10 @@
 from apptools.preferences.api import Preferences, PreferencesHelper
 from apptools.preferences.api import ScopedPreferences
 from apptools.preferences.api import set_default_preferences
-from traits.api import Any, Bool, HasTraits, Int, Float, List, Str
+from traits.api import (
+    Any, Bool, HasTraits, Int, Float, List, Str,
+    push_exception_handler, pop_exception_handler,
+)
 
 
 def width_listener(obj, trait_name, old, new):
@@ -60,6 +63,12 @@ def setUp(self):
         # A temporary directory that can safely be written to.
         self.tmpdir = tempfile.mkdtemp()
 
+        # Path to a temporary file
+        self.tmpfile = os.path.join(self.tmpdir, "tmp.ini")
+
+        push_exception_handler(reraise_exceptions=True)
+        self.addCleanup(pop_exception_handler)
+
     def tearDown(self):
         """ Called immediately after each test method has been called. """
 
@@ -255,6 +264,61 @@ class AcmeUIPreferencesHelper(PreferencesHelper):
         self.assertEqual("True", p.get("acme.ui.visible"))
         self.assertTrue(helper.visible)
 
+    def test_mutate_list_of_values(self):
+        """ Mutated list should be saved and _items events not to be
+        saved in the preferences.
+        """
+        # Regression test for enthought/apptools#129
+
+        class MyPreferencesHelper(PreferencesHelper):
+            preferences_path = Str('my_section')
+
+            list_of_str = List(Str)
+
+        helper = MyPreferencesHelper(list_of_str=["1"])
+
+        # Now modify the list to fire _items event
+        helper.list_of_str.append("2")
+        self.preferences.save(self.tmpfile)
+
+        new_preferences = Preferences()
+        new_preferences.load(self.tmpfile)
+
+        self.assertEqual(
+            new_preferences.get("my_section.list_of_str"), str(["1", "2"])
+        )
+        self.assertEqual(new_preferences.keys("my_section"), ["list_of_str"])
+
+    def test_nested_container_mutation_not_supported(self):
+        """ Known limitation: mutation on nested containers are not
+        synchronized
+        See enthought/apptools#194
+        """
+
+        class MyPreferencesHelper(PreferencesHelper):
+            preferences_path = Str('my_section')
+            list_of_list_of_str = List(List(Str))
+
+        helper = MyPreferencesHelper(list_of_list_of_str=[["1"]])
+        helper.list_of_list_of_str[0].append("9")
+
+        self.preferences.save(self.tmpfile)
+
+        new_preferences = Preferences()
+        new_preferences.load(self.tmpfile)
+
+        # The event trait is not saved.
+        self.assertEqual(
+            new_preferences.keys("my_section"), ["list_of_list_of_str"]
+        )
+
+        # The values are not synchronized
+        with self.assertRaises(AssertionError):
+            self.assertEqual(
+                new_preferences.get("my_section.list_of_list_of_str"),
+                str(helper.list_of_list_of_str)
+            )
+
     def test_no_preferences_path(self):
         """ no preferences path """
 

apptools/preferences/ui/tests/test_preferences_page.py

@@ -0,0 +1,77 @@
+""" Tests for the preferences page. """
+
+import unittest
+
+from traits.api import Enum, List, Str
+from traitsui.api import Group, Item, View
+
+from apptools.preferences.api import Preferences
+from apptools.preferences.ui.api import PreferencesPage
+
+
+class TestPreferencesPage(unittest.TestCase):
+    """ Non-GUI Tests for PreferencesPage."""
+
+    def test_preferences_page_apply(self):
+        """ Test applying the preferences """
+
+        # this sets up imitate Mayavi usage.
+
+        class MyPreferencesPage(PreferencesPage):
+
+            # the following set default values for class traits
+            category = "Application"
+
+            help_id = ""
+
+            name = "Note"
+
+            preferences_path = "my_ref.pref"
+
+            # custom preferences
+
+            backend = Enum("auto", "simple", "test")
+
+            traits_view = View(Group(Item("backend")))
+
+        preferences = Preferences()
+        pref_page = MyPreferencesPage(
+            preferences=preferences,
+            category="Another Application",
+            help_id="this_wont_be_saved",
+            name="Different Note",
+            # custom preferences
+            backend="simple",
+        )
+        pref_page.apply()
+
+        self.assertEqual(preferences.get("my_ref.pref.backend"), "simple")
+        self.assertEqual(preferences.keys("my_ref.pref"), ["backend"])
+
+        # this is not saved by virtue of it being static and never assigned to
+        self.assertIsNone(preferences.get("my_ref.pref.traits_view"))
+
+        # These are skipped because this trait is defined on the
+        # PreferencesPage.
+        self.assertIsNone(preferences.get("my_ref.pref.help_id"))
+        self.assertIsNone(preferences.get("my_ref.pref.category"))
+        self.assertIsNone(preferences.get("my_ref.pref.name"))
+
+    def test_preferences_page_apply_skip_items_traits(self):
+        """ Test _items traits from List mutation are skipped. """
+        # Regression test for enthought/apptools#129
+
+        class MyPreferencesPage(PreferencesPage):
+            preferences_path = "my_ref.pref"
+            names = List(Str())
+
+        preferences = Preferences()
+        pref_page = MyPreferencesPage(
+            preferences=preferences,
+            names=["1"],
+        )
+        pref_page.names.append("2")
+        pref_page.apply()
+
+        self.assertEqual(preferences.get("my_ref.pref.names"), str(["1", "2"]))
+        self.assertEqual(preferences.keys("my_ref.pref"), ["names"])

docs/releases/upcoming/196.bugfix.rst

@@ -0,0 +1 @@
+Fix container items change event being saved in preferences (#196)