diff --git a/apptools/preferences/preferences_helper.py b/apptools/preferences/preferences_helper.py index 2a8106dd1..e0a57f1a1 100644 --- a/apptools/preferences/preferences_helper.py +++ b/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 diff --git a/apptools/preferences/tests/test_preferences_helper.py b/apptools/preferences/tests/test_preferences_helper.py index 46f9cb38c..69622347e 100644 --- a/apptools/preferences/tests/test_preferences_helper.py +++ b/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 """ diff --git a/apptools/preferences/ui/tests/__init__.py b/apptools/preferences/ui/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/apptools/preferences/ui/tests/test_preferences_page.py b/apptools/preferences/ui/tests/test_preferences_page.py new file mode 100644 index 000000000..f1e01c183 --- /dev/null +++ b/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"]) diff --git a/docs/releases/upcoming/196.bugfix.rst b/docs/releases/upcoming/196.bugfix.rst new file mode 100644 index 000000000..0c12dfeb0 --- /dev/null +++ b/docs/releases/upcoming/196.bugfix.rst @@ -0,0 +1 @@ +Fix container items change event being saved in preferences (#196) \ No newline at end of file