Add settings versions and migrations

Author: astaric

Orange/widgets/settings.py

@@ -47,6 +47,8 @@
 
 _IMMUTABLES = (str, int, bytes, bool, float, tuple)
 
+VERSION_KEY = "__version__"
+
 
 class Setting:
     """Description of a setting.
@@ -386,6 +388,7 @@ def read_defaults_file(self, settings_file):
             for key, value in defaults.items()
             if not isinstance(value, Setting)
         }
+        self._migrate_settings(self.defaults)
 
     def write_defaults(self):
         """Write (global) defaults for this widget class to a file.
@@ -434,12 +437,18 @@ def initialize(self, instance, data=None):
 
         if isinstance(data, bytes):
             data = pickle.loads(data)
+        self._migrate_settings(data)
 
         if provider is self.provider:
             data = self._add_defaults(data)
 
         provider.initialize(instance, data)
 
+    def _migrate_settings(self, settings):
+        """Ask widget to migrate settings to the latest version."""
+        if settings:
+            self.widget_class.migrate_settings(settings, settings.pop(VERSION_KEY, None))
+
     def _select_provider(self, instance):
         provider = self.provider.get_provider(instance.__class__)
         if provider is None:
@@ -473,7 +482,9 @@ def pack_data(self, widget):
         ----------
         widget : OWWidget
         """
-        return self.provider.pack(widget)
+        packed_settings = self.provider.pack(widget)
+        packed_settings[VERSION_KEY] = self.widget_class.settings_version
+        return packed_settings
 
     def update_defaults(self, widget):
         """
@@ -588,6 +599,7 @@ def initialize(self, instance, data=None):
         super().initialize(instance, data)
         if data and "context_settings" in data:
             instance.context_settings = data["context_settings"]
+            self._migrate_contexts(instance.context_settings)
         else:
             instance.context_settings = []
 
@@ -596,16 +608,23 @@ def read_defaults_file(self, settings_file):
            pickle."""
         super().read_defaults_file(settings_file)
         self.global_contexts = pickle.load(settings_file)
+        self._migrate_contexts(self.global_contexts)
+
+    def _migrate_contexts(self, contexts):
+        for context in contexts:
+            self.widget_class.migrate_context(context, context.values.pop(VERSION_KEY, None))
 
     def write_defaults_file(self, settings_file):
         """Call the inherited method, then add global context to the pickle."""
         super().write_defaults_file(settings_file)
         pickle.dump(self.global_contexts, settings_file, -1)
 
     def pack_data(self, widget):
-        """Call the inherited method, then add local contexts to the pickle."""
+        """Call the inherited method, then add local contexts to the dict."""
         data = super().pack_data(widget)
         self.settings_from_widget(widget)
+        for context in widget.context_settings:
+            context.values[VERSION_KEY] = self.widget_class.settings_version
         data["context_settings"] = widget.context_settings
         return data
 

Orange/widgets/tests/test_context_handler.py

@@ -1,21 +1,78 @@
-from copy import copy
+import pickle
+from copy import copy, deepcopy
+from io import BytesIO
 from unittest import TestCase
-from unittest.mock import Mock
-from Orange.widgets.settings import ContextHandler, Setting, ContextSetting, Context
+from unittest.mock import Mock, patch, call
+from Orange.widgets.settings import ContextHandler, Setting, ContextSetting, Context, VERSION_KEY
 
 __author__ = 'anze'
 
 
 class SimpleWidget:
+    settings_version = 1
+
     setting = Setting(42)
 
     context_setting = ContextSetting(42)
 
+    migrate_settings = Mock()
+    migrate_context = Mock()
+
+
+class DummyContext(Context):
+    id = 0
+
+    def __init__(self, version=None):
+        super().__init__()
+        DummyContext.id += 1
+        self.id = DummyContext.id
+        if version:
+            self.values[VERSION_KEY] = version
+
+    def __repr__(self):
+        return "Context(id={})".format(self.id)
+    __str__ = __repr__
+
+    def __eq__(self, other):
+        if not isinstance(other, DummyContext):
+            return False
+        return self.id == other.id
+
+
+def create_defaults_file(contexts):
+    b = BytesIO()
+    pickle.dump({"x": 5}, b)
+    pickle.dump(contexts, b)
+    b.seek(0)
+    return b
+
 
 class TestContextHandler(TestCase):
+    def test_read_defaults(self):
+        contexts = [DummyContext() for _ in range(3)]
+
+        handler = ContextHandler()
+        handler.widget_class = SimpleWidget
+
+        # Old settings without version
+        migrate_context = Mock()
+        with patch.object(SimpleWidget, "migrate_context", migrate_context):
+            handler.read_defaults_file(create_defaults_file(contexts))
+        self.assertSequenceEqual(handler.global_contexts, contexts)
+        migrate_context.assert_has_calls([call(c, None) for c in contexts])
+
+        # Settings with version
+        contexts = [DummyContext(version=i) for i in range(1, 4)]
+        migrate_context.reset_mock()
+        with patch.object(SimpleWidget, "migrate_context", migrate_context):
+            handler.read_defaults_file(create_defaults_file(contexts))
+        self.assertSequenceEqual(handler.global_contexts, contexts)
+        migrate_context.assert_has_calls([call(c, c.values[VERSION_KEY]) for c in contexts])
+
     def test_initialize(self):
         handler = ContextHandler()
         handler.provider = Mock()
+        handler.widget_class = SimpleWidget
 
         # Context settings from data
         widget = SimpleWidget()
@@ -29,6 +86,26 @@ def test_initialize(self):
         self.assertTrue(hasattr(widget, 'context_settings'))
         self.assertEqual(widget.context_settings, handler.global_contexts)
 
+    def test_initialize_migrates_contexts(self):
+        handler = ContextHandler()
+        handler.bind(SimpleWidget)
+
+        widget = SimpleWidget()
+
+        # Old settings without version
+        contexts = [DummyContext() for _ in range(3)]
+        migrate_context = Mock()
+        with patch.object(SimpleWidget, "migrate_context", migrate_context):
+            handler.initialize(widget, dict(context_settings=contexts))
+        migrate_context.assert_has_calls([call(c, None) for c in contexts])
+
+        # Settings with version
+        contexts = [DummyContext(version=i) for i in range(1, 4)]
+        migrate_context = Mock()
+        with patch.object(SimpleWidget, "migrate_context", migrate_context):
+            handler.initialize(widget, dict(context_settings=deepcopy(contexts)))
+        migrate_context.assert_has_calls([call(c, c.values[VERSION_KEY]) for c in contexts])
+
     def test_fast_save(self):
         handler = ContextHandler()
         handler.bind(SimpleWidget)
@@ -68,3 +145,16 @@ def test_find_or_create_context(self):
         self.assertEqual(context.i, 5)
         self.assertEqual([c.i for c in widget.context_settings], [5, 2])
         self.assertEqual([c.i for c in handler.global_contexts], [3, 7])
+
+    def test_pack_settings_stores_version(self):
+        handler = ContextHandler()
+        handler.bind(SimpleWidget)
+
+        widget = SimpleWidget()
+        handler.initialize(widget)
+        widget.context_setting = [DummyContext() for _ in range(3)]
+
+        settings = handler.pack_data(widget)
+        self.assertIn("context_settings", settings)
+        for c in settings["context_settings"]:
+            self.assertIn(VERSION_KEY, c.values)

Orange/widgets/tests/test_settings_handler.py

@@ -5,7 +5,7 @@
 import unittest
 from unittest.mock import patch, Mock, mock_open
 import warnings
-from Orange.widgets.settings import SettingsHandler, Setting, SettingProvider
+from Orange.widgets.settings import SettingsHandler, Setting, SettingProvider, VERSION_KEY
 
 
 class SettingHandlerTestCase(unittest.TestCase):
@@ -211,6 +211,57 @@ def test_schema_only_settings(self):
         data = handler.pack_data(widget)
         self.assertEqual(data['schema_only_setting'], 5)
 
+    def test_read_defaults_migrates_settings(self):
+        handler = SettingsHandler()
+        handler.widget_class = SimpleWidget
+
+        migrate_settings = Mock()
+        with patch.object(SimpleWidget, "migrate_settings", migrate_settings):
+            # Old settings without version
+            settings = {"value": 5}
+            with self.override_defaults(settings):
+                handler.read_defaults()
+            migrate_settings.assert_called_with(settings, None)
+
+            migrate_settings.reset()
+            # Settings with version
+            settings_with_version = dict(settings)
+            settings_with_version[VERSION_KEY] = 1
+            with self.override_defaults(settings_with_version):
+                handler.read_defaults()
+            migrate_settings.assert_called_with(settings, 1)
+
+    def test_initialize_migrates_settings(self):
+        handler = SettingsHandler()
+        with self.override_defaults():
+            handler.bind(SimpleWidget)
+
+        widget = SimpleWidget()
+
+        migrate_settings = Mock()
+        with patch.object(SimpleWidget, "migrate_settings", migrate_settings):
+            # Old settings without version
+            settings = {"value": 5}
+
+            handler.initialize(widget, settings)
+            migrate_settings.assert_called_with(settings, None)
+
+            migrate_settings.reset_mock()
+            # Settings with version
+
+            settings_with_version = dict(settings)
+            settings_with_version[VERSION_KEY] = 1
+            handler.initialize(widget, settings_with_version)
+            migrate_settings.assert_called_with(settings, 1)
+
+    def test_pack_settings_stores_version(self):
+        handler = SettingsHandler()
+        handler.bind(SimpleWidget)
+
+        widget = SimpleWidget()
+
+        settings = handler.pack_data(widget)
+        self.assertIn(VERSION_KEY, settings)
 
     @contextmanager
     def override_defaults(self, defaults=None):
@@ -227,6 +278,8 @@ class Component:
 
 
 class SimpleWidget:
+    settings_version = 1
+
     setting = Setting(42)
     schema_only_setting = Setting(None, schema_only=True)
     non_setting = 5
@@ -236,6 +289,9 @@ class SimpleWidget:
     def __init__(self):
         self.component = Component()
 
+    migrate_settings = Mock()
+    migrate_context = Mock()
+
 
 class SimpleWidgetMk1(SimpleWidget):
     pass

Orange/widgets/widget.py

@@ -154,6 +154,13 @@ class OWWidget(QDialog, Report, ProgressBarMixin, WidgetMessagesMixin,
     settingsHandler = None
     """:type: SettingsHandler"""
 
+    #: Version of the settings representation
+    #: Subclasses should increase this number when they make breaking
+    #: changes to settings representation (a settings that used to store
+    #: int now stores string) and handle migrations in migrate and
+    #: migrate_context settings.
+    settings_version = 1
+
     savedWidgetGeometry = settings.Setting(None)
 
     #: A list of advice messages (:class:`Message`) to display to the user.
@@ -738,6 +745,32 @@ def _userconfirmed():
 
         self.__msgwidget.accepted.connect(_userconfirmed)
 
+    @classmethod
+    def migrate_settings(cls, settings, version):
+        """Fix settings to work with the current version of widgets
+
+        Parameters
+        ----------
+        settings : dict
+            dict of name - value mappings
+        version : Optional[int]
+            version of the saved settings
+            or None if settings were created before migrations
+        """
+
+    @classmethod
+    def migrate_context(cls, context, version):
+        """Fix contexts to work with the current version of widgets
+
+        Parameters
+        ----------
+        context : Context
+            Context object
+        version : Optional[int]
+            version of the saved context
+            or None if context was created before migrations
+        """
+
 
 class Message(object):
     """